UniverseUniversity


Home Projects Jobs Clientele Contact

uu


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: User registration/authentication



Well, as an expert in C++ I can tell that's how I do casting from string
to long.
There is no asLong() function in C++ as much as I know, what's the right
way to do it?

> Can anyone tell me, why do we need this cast:
>
>     sinfo.person_id = lexical_cast<long>(idField.asString());
>
>
> 2007/3/17, Ilya A. Volynets-Evenbakh <ilya@total-knowledge.com>:
>>
>> sergey@total-knowledge.com wrote:
>> > Attached trunk0316.diff file with current application.
>> >
>> > It has Register, Login and Home pages. Home page is login protected
>> and
>> > not-logged-in user gets redirected to Login page.
>> >
>> Why?
>> >
>> > Index: libui/RegisterServlet.h
>> > ===================================================================
>> > --- libui/RegisterServlet.h   (revision 0)
>> > +++ libui/RegisterServlet.h   (revision 0)
>> > @@ -0,0 +1,28 @@
>> > +#ifndef REGISTERSERVLET_H
>> > +#define REGISTERSERVLET_H
>> > +
>> > +#include <boost/lexical_cast.hpp>
>> > +#include "UUServlet.h"
>> > +
>> > +using boost::lexical_cast;
>> > +using namespace std;
>> > +using namespace servlet;
>> >
>> Never put "using" statements in header files.
>> > +
>> > +/**
>> > +  * This servlet:
>> > +  * Receives registration request.
>> > +  * Accesses User model object.
>> > +  * Stores results in request context.
>> > +  * Determines the language to render page in based on user
>> preferences.
>> > +  * Forwards the request to the view object (HomeView.csp) if no
>> errors.
>> >
>> Shouldn't RegisterServlet be using something like Register.csp?
>> > Index: libui/UUServlet.cpp
>> > ===================================================================
>> > --- libui/UUServlet.cpp       (revision 0)
>> > +++ libui/UUServlet.cpp       (revision 0)
>> > @@ -0,0 +1,48 @@
>> > +#include "UUServlet.h"
>> > +
>> > +UUServlet::UUServlet ( ) { }
>> >
>> Do not create empty constructors.
>> > +UUServlet::~UUServlet ( ) { }
>> >
>> Only create empty destructors if they are virtual (so, it's OK in this
>> case).
>> > +
>> > +/**
>> > + * Returns true if the request already has valid associated session,
>> and that
>> > + * session contains valid User object
>> > + * @return bool
>> > + * @param  req Request to check
>> > + */
>> > +
>> > +bool UUServlet::isUserLoggedIn (HttpServletRequest& req ) const {
>> >
>> Put opening '{' on new line for functions (not for blocks inside
>> functions though)
>> > +
>> > +  HttpSession* session = req.getSession(true);
>> > +  if(!session) throw runtime_error("no session");
>> >
>> That error message doesn't make that much sense.
>> > +  string sessionid = session->getId();
>> > +  user_t user;
>> > +  bool hasValidLogin = user.isSessionValid(sessionid);
>> >
>> Session validity is not defined by whether user is logged in or not,
>> so I guess that's a misnomer.
>> > +  return hasValidLogin;
>> > +
>> > +}
>> > +
>> > +
>> > +/**
>> > + * Redirects browser to login page
>> > + * @param  resp send redirect through this response object
>> > + */
>> > +void UUServlet::redirectToLogin (HttpServletRequest& req,
>> HttpServletResponse& resp) {
>> > +
>> > +  req.setAttribute("Message", setattr_t(new string("Please login
>> first!")));
>> >
>> What does setattr_t mean? (Again - load your names with real meaning)
>> Another thing - why are you specifying that exact message? Couldn't it
>> so happen
>> that you are redirecting to the login page from somewhere where no log
>> in
>> is
>> required (LogoutServlet perhaps?)
>> > +  req.getRequestDispatcher("LoginView.csp")->forward(req,resp);
>> > +
>> > +}
>> > +
>> > +
>> > +/**
>> > + * @return User
>> > + * @param  req
>> > + */
>> > +User UUServlet::getCurrentUser (HttpServletRequest& req ) {
>> > +
>> > +  user_t user;
>> > +  return user;
>> >
>> Eh? If it isn't implemented - leave //FIXME: or //TODO: marker.
>> Otherwise, comment this piece of crypto properly.
>> > +
>> > +}
>> > +
>> > +
>> >
>> >
>> > Index: libui/LoginServlet.h
>> > ===================================================================
>> > --- libui/LoginServlet.h      (revision 0)
>> > +++ libui/LoginServlet.h      (revision 0)
>> > @@ -0,0 +1,25 @@
>> > +#ifndef LOGINSERVLET_H
>> > +#define LOGINSERVLET_H
>> > +
>> > +#include "UUServlet.h"
>> > +
>> > +using namespace std;
>> > +using namespace servlet;
>> > +
>> > +/**
>> > +  *  This servlet:
>> > +  *  Receives login request.
>> > +  *  Accesses User model object.
>> > +  *  Stores results in request context.
>> > +  *  Determines the language to render page in based on user
>> preferences.
>> > +  *  Forwards the request to the view object (HomeView.csp) if no
>> errors.
>> > +  */
>> > +
>> > +class LoginServlet : public UUServlet
>> > +{
>> > +public:
>> > +  void service(HttpServletRequest& request, HttpServletResponse
>> &response);
>> > +};
>> > +
>> > +#endif//LOGINSERVLET_H
>> > +
>> >
>> >
>> > Index: libui/UUServlet.h
>> > ===================================================================
>> > --- libui/UUServlet.h (revision 0)
>> > +++ libui/UUServlet.h (revision 0)
>> > @@ -0,0 +1,57 @@
>> > +#ifndef UUSERVLET_H
>> > +#define UUSERVLET_H
>> > +class UUServlet : public servlet::HttpServlet
>> > +{
>> > +public:
>> > +  /**
>> > +   * Redirects browser to login page
>> > +   * @param  resp send redirect through this response object
>> > +   */
>> > +  void redirectToLogin (servlet::HttpServletRequest& req,
>> servlet::HttpServletResponse& resp);
>> >
>> Do you need request arg here?
>> >
>> > Index: libui/Makefile.adon
>> > ===================================================================
>> > --- libui/Makefile.adon       (revision 82)
>> > +++ libui/Makefile.adon       (working copy)
>> > +noinst_HEADERS:=../libdm/User.h
>> Can't do that. Only files current dir can be mentioned.
>> > +
>> > +EXTRA_DIST:=Footer.csp Header.csp Sidebar.csp
>> > +libuu_SOURCES:=../libdm/User.cpp UUServlet.cpp ../libdm/uudb.cpp
>> >
>> Again. Can't do that. libdm should be a separate library, that you will
>> link with.
>> >
>> > Index: engine.xml
>> > ===================================================================
>> > --- engine.xml        (revision 0)
>> > +++ engine.xml        (revision 0)
>> > @@ -0,0 +1,10 @@
>> > +<?xml version="1.0"?>
>> > +<listener protocol="unix" path="/tmp/cppserv.sock"/>
>> > +<app name="">
>> > +    <servlet name="HomeServlet" dso="./debug/libui/HomeServlet.so"/>
>> > +    <servlet name="RegisterServlet"
>> dso="./debug/libui/RegisterServlet.so"/>
>> > +    <servlet name="LoginServlet"
>> dso="./debug/libui/LoginServlet.so"/>
>> > +    <csp name="themes/HomeView.csp" path="HomeView.csp"
>> dso="./debug/themes/HomeView.so" hidden="true"/>
>> > +    <csp name="themes/RegistrationView.csp"
>> path="RegistrationView.csp"
>> dso="./debug/themes/RegistrationView.so" hidden="true"/>
>> > +    <csp name="themes/LoginView.csp" path="LoginView.csp"
>> dso="./debug/themes/LoginView.so" hidden="true"/>
>> > +</app>
>> >
>> You know that cppserv now supports multiple servlets in single DSO,
>> right?
>> i.e. you could link them all into a single library.. That might make
>> some things easier
>> (not necessary by all means, but probably good idea for themes).
>> Speaking of themes, make sure your templates are in subdirectory of
>> themes, not
>> in themes directory itself. something like themes/default/
>> > Index: libdm/User.cpp
>> > ===================================================================
>> > --- libdm/User.cpp    (revision 0)
>> > +++ libdm/User.cpp    (revision 0)
>> > @@ -0,0 +1,128 @@
>> > +#include "User.h"
>> > +
>> > +User::User() {}
>> > +User::~User() {}
>> > +
>> > +/**
>> > + * Performs authentication.
>> > + * @return bool
>> > + */
>> > +
>> > +bool User::isLoginPasswordValid(string login, string password) {
>> > +
>> > +  CODBCDatabase db("DSN=PostgreSQL;UID=sergey;PWD=;DATABASE=uu");
>> > +  bool isValid = false;
>> > +  try {
>> > +    CQuery qrySelect(&db,"select pl_id from person_list where
>> pl_login=:login and pl_password=:password");
>> > +    qrySelect.param("login") = login;
>> > +    qrySelect.param("password") = password;
>> > +    qrySelect.open();
>> > +    while ( ! qrySelect.eof() ) {
>> > +      isValid = true;
>> > +      qrySelect.fetch();
>> > +    }
>> > +    qrySelect.close();
>> > +  }
>> > +  catch (exception& e) {
>> > +    cout<<"\nError: " <<e.what();
>> > +    return false;
>> > +  }
>> > +  return isValid;
>> > +
>> > +}
>> > +
>> > +/**
>> > + * Sets data to session_info temp table after successful
>> authentication
>> > + */
>> > +
>> > +bool User::setSessionInfo(string login, string password, int server,
>> string sessionid) {
>> > +
>> > +  CODBCDatabase db("DSN=PostgreSQL;UID=sergey;PWD=;DATABASE=uu");
>> > +  try {
>> > +    CQuery qryInsert(&db,"select login('"+login+"', '"+password+"',
>> "+lexical_cast<string>(server)+")");
>> > +    qryInsert.exec();
>> > +  }
>> > +  catch (exception& e) {
>> > +    cout<<"\nError: " <<e.what();
>> > +    return false;
>> > +  }
>> > +  return true;
>> > +
>> > +}
>> > +
>> > +/**
>> > + * Gets user data from session_info table.
>> > + */
>> > +
>> > +User::sessioninfo_t User::getSessionInfo(string sessionid) {
>> > +
>> > +  CODBCDatabase db("DSN=PostgreSQL;UID=sergey;PWD=;DATABASE=uu");
>> > +  sessioninfo_t sessioninfo;
>> > +  try {
>> > +    CQuery qrySelect(&db,"select * from session_info where
>> si_session=:sessionid");
>> > +    qrySelect.param("sessionid") = sessionid;
>> > +    qrySelect.open();
>> > +    CField& idField = qrySelect["si_person"];
>> > +    CField& fnameField = qrySelect["si_person_name"];
>> > +    CField& serverField = qrySelect["si_server"];
>> > +    while ( ! qrySelect.eof() ) {
>> > +      SessionInfo sinfo;
>> > +      sinfo.person_id = lexical_cast<long>(idField.asString());
>> > +      sinfo.person_name = fnameField.asString();
>> > +      sinfo.server = serverField.asInteger();
>> > +      sessioninfo.push_back(sinfo);
>> > +      qrySelect.fetch();
>> > +    }
>> > +    qrySelect.close();
>> > +  }
>> > +  catch (exception& e) {
>> > +    cout<<"\nError: " <<e.what();
>> > +  }
>> > +  return sessioninfo;
>> > +
>> > +}
>> > +
>> > +/**
>> > + * Gets user id from session_info table. Returns true if such user
>> exist.
>> > + */
>> > +
>> > +bool User::isSessionValid(string sessionid) {
>> > +
>> > +  CODBCDatabase db("DSN=PostgreSQL;UID=sergey;PWD=;DATABASE=uu");
>> > +  bool isValid = false;
>> > +  try {
>> > +    CQuery qrySelect(&db,"select si_person from session_info where
>> si_session=:sessionid");
>> > +    qrySelect.param("sessionid") = sessionid;
>> > +    qrySelect.open();
>> > +    while ( ! qrySelect.eof() ) {
>> > +      isValid = true;
>> > +      qrySelect.fetch();
>> > +    }
>> > +    qrySelect.close();
>> > +  }
>> > +  catch (exception& e) {
>> > +    cout<<"\nError: " <<e.what();
>> > +  }
>> > +  return isValid;
>> > +
>> > +}
>> > +
>> > +/**
>> > + * Sets all user data during a registration process. Calls stored
>> procedure "person_create" to insert user data into "person_list" table.
>> If
>> insert is successful, returns true. Otherwise returns false.
>> > + */
>> > +
>> > +bool User::setUserInfo(string login, string password, int language,
>> string firstName, string lastName, string street, string city, string
>> state,
>> string zip, string country, string email, string phone ) {
>> > +
>> > +  CODBCDatabase db("DSN=PostgreSQL;UID=sergey;PWD=;DATABASE=uu");
>> > +  try {
>> > +    CQuery qryInsert(&db,"select
>> person_create('"+firstName+"','"+lastName+"','"+street+"','"+city+"','"+state+"','"+zip+"','"+country+"','"+email+"','"+phone+"','"+login+"','"+password+"')");
>> > +    qryInsert.exec();
>> > +  }
>> > +  catch (exception& e) {
>> > +    cout<<"\nError: " <<e.what();
>> > +    return false;
>> > +  }
>> > +  return true;
>> > +
>> > +}
>> > +
>> >
>> >
>> I will let Alexey review and comment your database code.
>> >
>> > Index: libdm/uudb.cpp
>> > ===================================================================
>> > --- libdm/uudb.cpp    (revision 0)
>> > +++ libdm/uudb.cpp    (revision 0)
>> > @@ -0,0 +1,33 @@
>> >
>> All of the uudb code you wrote should be replaced with db pool code.
>>
>>
>> I didn't review some of your actual code, for now fixing issues that
>> has already been pointed out will be enough for next iteration.
>>
>> --
>> Ilya A. Volynets-Evenbakh
>> Total Knowledge. CTO
>> http://www.total-knowledge.com
>>
>>
>
>
> --
> Alexey Parshin,
> http://www.sptk.net
>



Authoright © Total Knowledge: 2001-2008