[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
Re: User registration/authentication
It worked!!!
Wow, I'm impressed with SPTK. It knows how I declare my variables and does
casting by itself!
> Actually,
>
> It should be possible to use just:
>
> sinfo.person_id = idField;
>
> 2007/3/17, sergey@total-knowledge.com <sergey@total-knowledge.com>:
>>
>> I had it initially as
>> sinfo.person_id = idField.asInteger();
>>
>> But I thought it's a wrong way to do it since person_id is type 'long'
>> in
>> SessionInfo vector.
>>
>> > 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
>> >>
>> >
>> >
>> >
>>
>>
>>
>
>
> --
> Alexey Parshin,
> http://www.sptk.net
>