Fixed user registration activation workflow.

- Fixed new user insertion in SQL database.
 - Removed temp user when activated.
 - Renamed local variable names related to temp users.
 - More information when error occurs during user registration.
This commit is contained in:
Jocelyn Fiat
2017-04-10 21:51:41 +02:00
parent 85ac9f7366
commit 3dc478b4a0
4 changed files with 72 additions and 71 deletions

View File

@@ -445,44 +445,52 @@ feature -- Handler
l_user_api: CMS_USER_API l_user_api: CMS_USER_API
l_ir: INTERNAL_SERVER_ERROR_CMS_RESPONSE l_ir: INTERNAL_SERVER_ERROR_CMS_RESPONSE
es: CMS_AUTHENTICATION_EMAIL_SERVICE es: CMS_AUTHENTICATION_EMAIL_SERVICE
l_temp_id: INTEGER_64
do do
l_user_api := api.user_api if api.has_permission ("account activate") then
create {GENERIC_VIEW_CMS_RESPONSE} r.make (req, res, api) l_user_api := api.user_api
if r.has_permission ("account activate") then
if attached {WSF_STRING} req.path_parameter ("token") as l_token then if attached {WSF_STRING} req.path_parameter ("token") as l_token then
if attached {CMS_TEMP_USER} l_user_api.temp_user_by_activation_token (l_token.value) as l_user then if attached {CMS_TEMP_USER} l_user_api.temp_user_by_activation_token (l_token.value) as l_temp_user then
-- TODO copy the personal information -- TODO copy the personal information
--! to CMS_USER_PROFILE and persist data --! to CMS_USER_PROFILE and persist data
--! check also CMS_USER.data_items --! check also CMS_USER.data_items
-- Valid user_id l_temp_id := l_temp_user.id
l_user.set_id (0)
l_user.mark_active
l_user_api.new_user_from_temp_user (l_user)
-- Valid user_id
l_temp_user.set_id (0)
l_temp_user.mark_active
l_user_api.new_user_from_temp_user (l_temp_user)
create {GENERIC_VIEW_CMS_RESPONSE} r.make (req, res, api)
if if
not l_user_api.has_error and then not l_user_api.has_error and then
attached l_user_api.user_by_name (l_user.name) as l_new_user attached l_user_api.user_by_name (l_temp_user.name) as l_new_user
then then
-- Delete temporal User -- Delete temporal User
l_user_api.delete_temp_user (l_user) l_temp_user.set_id (l_temp_id)
l_user_api.delete_temp_user (l_temp_user)
l_user_api.remove_activation (l_token.value) l_user_api.remove_activation (l_token.value)
r.set_main_content ("<p> The account <i>" + html_encoded (l_user.name) + "</i> has been activated</p>") r.set_main_content ("<p> The account <i>" + html_encoded (l_new_user.name) + "</i> has been activated</p>")
-- Send Email -- Send Email
if attached l_user.email as l_email then if attached l_new_user.email as l_email then
create es.make (create {CMS_AUTHENTICATION_EMAIL_SERVICE_PARAMETERS}.make (api)) create es.make (create {CMS_AUTHENTICATION_EMAIL_SERVICE_PARAMETERS}.make (api))
write_debug_log (generator + ".handle register: send_contact_activation_confirmation_email") write_debug_log (generator + ".handle register: send_contact_activation_confirmation_email")
es.send_contact_activation_confirmation_email (l_email, l_user, req.absolute_script_url ("")) es.send_contact_activation_confirmation_email (l_email, l_new_user, req.absolute_script_url (""))
end end
else else
-- Failure!!! -- Failure!!!
r.set_status_code ({HTTP_CONSTANTS}.internal_server_error) r.set_status_code ({HTTP_CONSTANTS}.internal_server_error)
r.set_main_content ("<p>ERROR: User activation failed for <i>" + html_encoded (l_user.name) + "</i>!</p>") r.set_main_content ("<p>ERROR: User activation failed for <i>" + html_encoded (l_temp_user.name) + "</i>!</p>")
if attached l_user_api.error_handler.as_single_error as err then
r.add_error_message (html_encoded (err.string_representation))
end
end end
else else -- the token does not exist, or it was already used.
-- the token does not exist, or it was already used. create {GENERIC_VIEW_CMS_RESPONSE} r.make (req, res, api)
r.set_status_code ({HTTP_CONSTANTS}.bad_request) r.set_status_code ({HTTP_CONSTANTS}.bad_request)
r.set_main_content ("<p>The token <i>" + l_token.value + "</i> is not valid " + r.link ("Reactivate Account", "account/reactivate", Void) + "</p>") r.set_main_content ("<p>The token <i>" + l_token.value + "</i> is not valid " + r.link ("Reactivate Account", "account/reactivate", Void) + "</p>")
end end
@@ -493,7 +501,7 @@ feature -- Handler
end end
else else
create {FORBIDDEN_ERROR_CMS_RESPONSE} r.make (req, res, api) create {FORBIDDEN_ERROR_CMS_RESPONSE} r.make (req, res, api)
r.execute r.execute
end end
end end

View File

@@ -241,7 +241,6 @@ feature -- Access: Temp Users
password: Result /= Void implies (Result.hashed_password /= Void and Result.password = Void) password: Result /= Void implies (Result.hashed_password /= Void and Result.password = Void)
end end
temp_user_by_activation_token (a_token: READABLE_STRING_32): detachable CMS_USER temp_user_by_activation_token (a_token: READABLE_STRING_32): detachable CMS_USER
-- User with activation token `a_token', if any. -- User with activation token `a_token', if any.
deferred deferred
@@ -254,19 +253,15 @@ feature -- Access: Temp Users
deferred deferred
end end
token_by_temp_user_id (a_id: like {CMS_USER}.id): detachable STRING token_by_temp_user_id (a_id: like {CMS_USER}.id): detachable STRING
-- Retrieve activation token for user identified with id `a_id', if any. -- Retrieve activation token for user identified with id `a_id', if any.
deferred deferred
end end
feature -- New Temp User feature -- New Temp User
new_user_from_temp_user (a_user: CMS_TEMP_USER) new_user_from_temp_user (a_temp_user: CMS_TEMP_USER)
-- new user from temporal user `a_user' -- new user from temporal user `a_temp_user'
require
no_id: not a_user.has_id
deferred deferred
end end
@@ -275,17 +270,17 @@ feature -- New Temp User
deferred deferred
end end
new_temp_user (a_user: CMS_TEMP_USER) new_temp_user (a_temp_user: CMS_TEMP_USER)
-- New temp user `a_user'. -- New temp user `a_temp_user'.
require require
no_id: not a_user.has_id no_id: not a_temp_user.has_id
deferred deferred
end end
delete_temp_user (a_user: CMS_TEMP_USER) delete_temp_user (a_temp_user: CMS_TEMP_USER)
-- Delete user `a_user'. -- Delete user `a_temp_user'.
require require
has_id: a_user.has_id has_id: a_temp_user.has_id
deferred deferred
end end

View File

@@ -217,13 +217,14 @@ feature -- Change: user
l_password_hash := l_security.password_hash (l_password, l_password_salt) l_password_hash := l_security.password_hash (l_password, l_password_salt)
write_information_log (generator + ".new_user") write_information_log (generator + ".new_user")
create l_parameters.make (4) create l_parameters.make (7)
l_parameters.put (a_user.name, "name") l_parameters.put (a_user.name, "name")
l_parameters.put (l_password_hash, "password") l_parameters.put (l_password_hash, "password")
l_parameters.put (l_password_salt, "salt") l_parameters.put (l_password_salt, "salt")
l_parameters.put (l_email, "email") l_parameters.put (l_email, "email")
l_parameters.put (create {DATE_TIME}.make_now_utc, "created") l_parameters.put (create {DATE_TIME}.make_now_utc, "created")
l_parameters.put (a_user.status, "status") l_parameters.put (a_user.status, "status")
l_parameters.put (a_user.profile_name, "profile_name")
sql_insert (sql_insert_user, l_parameters) sql_insert (sql_insert_user, l_parameters)
if not error_handler.has_error then if not error_handler.has_error then
@@ -1231,33 +1232,31 @@ feature {NONE} -- Implementation: User
feature -- New Temp User feature -- New Temp User
new_user_from_temp_user (a_user: CMS_TEMP_USER) new_user_from_temp_user (a_temp_user: CMS_TEMP_USER)
-- <Precursor> -- <Precursor>
local local
l_parameters: STRING_TABLE [detachable ANY] l_parameters: STRING_TABLE [detachable ANY]
do do
error_handler.reset error_handler.reset
if if
attached a_user.hashed_password as l_password_hash and then attached a_temp_user.hashed_password as l_password_hash and then
attached a_user.email as l_email and then attached a_temp_user.email as l_email and then
attached a_user.salt as l_password_salt attached a_temp_user.salt as l_password_salt
then then
-- FIXME: store the personal_information in profile! -- FIXME: store the personal_information in profile!
sql_begin_transaction sql_begin_transaction
write_information_log (generator + ".new_user_from_temp_user") write_information_log (generator + ".new_user_from_temp_user")
create l_parameters.make (6) create l_parameters.make (7)
l_parameters.put (a_user.name, "name") l_parameters.put (a_temp_user.name, "name")
l_parameters.put (l_password_hash, "password") l_parameters.put (l_password_hash, "password")
l_parameters.put (l_password_salt, "salt") l_parameters.put (l_password_salt, "salt")
l_parameters.put (l_email, "email") l_parameters.put (l_email, "email")
l_parameters.put (create {DATE_TIME}.make_now_utc, "created") l_parameters.put (create {DATE_TIME}.make_now_utc, "created")
l_parameters.put (a_user.status, "status") l_parameters.put (a_temp_user.status, "status")
l_parameters.put (a_temp_user.profile_name, "profile_name")
sql_insert (sql_insert_user, l_parameters) sql_insert (sql_insert_user, l_parameters)
if not error_handler.has_error then
a_user.set_id (last_inserted_user_id)
end
if not error_handler.has_error then if not error_handler.has_error then
sql_commit_transaction sql_commit_transaction
else else
@@ -1270,8 +1269,8 @@ feature -- New Temp User
end end
end end
new_temp_user (a_user: CMS_TEMP_USER) new_temp_user (a_temp_user: CMS_TEMP_USER)
-- Add a new temp_user `a_user'. -- Add a new temp_user `a_temp_user'.
local local
l_parameters: STRING_TABLE [detachable ANY] l_parameters: STRING_TABLE [detachable ANY]
l_password_salt, l_password_hash: STRING l_password_salt, l_password_hash: STRING
@@ -1279,9 +1278,9 @@ feature -- New Temp User
do do
error_handler.reset error_handler.reset
if if
attached a_user.password as l_password and then attached a_temp_user.password as l_password and then
attached a_user.email as l_email and then attached a_temp_user.email as l_email and then
attached a_user.personal_information as l_personal_information attached a_temp_user.personal_information as l_personal_information
then then
create l_security create l_security
@@ -1290,7 +1289,7 @@ feature -- New Temp User
write_information_log (generator + ".new_temp_user") write_information_log (generator + ".new_temp_user")
create l_parameters.make (4) create l_parameters.make (4)
l_parameters.put (a_user.name, "name") l_parameters.put (a_temp_user.name, "name")
l_parameters.put (l_password_hash, "password") l_parameters.put (l_password_hash, "password")
l_parameters.put (l_password_salt, "salt") l_parameters.put (l_password_salt, "salt")
l_parameters.put (l_email, "email") l_parameters.put (l_email, "email")
@@ -1299,7 +1298,7 @@ feature -- New Temp User
sql_begin_transaction sql_begin_transaction
sql_insert (sql_insert_temp_user, l_parameters) sql_insert (sql_insert_temp_user, l_parameters)
if not error_handler.has_error then if not error_handler.has_error then
a_user.set_id (last_inserted_temp_user_id) a_temp_user.set_id (last_inserted_temp_user_id)
sql_commit_transaction sql_commit_transaction
else else
sql_rollback_transaction sql_rollback_transaction
@@ -1328,8 +1327,8 @@ feature -- Remove Activation
sql_finalize sql_finalize
end end
delete_temp_user (a_user: CMS_TEMP_USER) delete_temp_user (a_temp_user: CMS_TEMP_USER)
-- Delete user `a_user'. -- Delete user `a_temp_user'.
local local
l_parameters: STRING_TABLE [detachable ANY] l_parameters: STRING_TABLE [detachable ANY]
do do
@@ -1337,7 +1336,7 @@ feature -- Remove Activation
sql_begin_transaction sql_begin_transaction
write_information_log (generator + ".delete_temp_user") write_information_log (generator + ".delete_temp_user")
create l_parameters.make (1) create l_parameters.make (1)
l_parameters.put (a_user.id, "uid") l_parameters.put (a_temp_user.id, "uid")
sql_modify (sql_delete_temp_user, l_parameters) sql_modify (sql_delete_temp_user, l_parameters)
sql_commit_transaction sql_commit_transaction
sql_finalize sql_finalize

View File

@@ -454,38 +454,37 @@ feature -- Access - Temp User
feature -- Change Temp User feature -- Change Temp User
new_user_from_temp_user (a_user: CMS_TEMP_USER) new_user_from_temp_user (a_temp_user: CMS_TEMP_USER)
-- Add a new user `a_user'. -- Add a new user `a_temp_user'.
require require
no_id: not a_user.has_id has_hashed_password: a_temp_user.hashed_password /= Void
has_hashed_password: a_user.hashed_password /= Void has_sal: a_temp_user.salt /= Void
has_sal: a_user.salt /= Void
do do
reset_error reset_error
if if
attached a_user.hashed_password as l_password and then attached a_temp_user.hashed_password as l_password and then
attached a_user.salt as l_salt and then attached a_temp_user.salt as l_salt and then
attached a_user.email as l_email attached a_temp_user.email as l_email
then then
storage.new_user_from_temp_user (a_user) storage.new_user_from_temp_user (a_temp_user)
error_handler.append (storage.error_handler) error_handler.append (storage.error_handler)
else else
error_handler.add_custom_error (0, "bad new user request", "Missing password or email to create new user!") error_handler.add_custom_error (0, "bad new user request", "Missing password or email to create new user!")
end end
end end
new_temp_user (a_user: CMS_TEMP_USER) new_temp_user (a_temp_user: CMS_TEMP_USER)
-- Add a new user `a_user'. -- Add a new user `a_temp_user'.
require require
no_id: not a_user.has_id no_id: not a_temp_user.has_id
no_hashed_password: a_user.hashed_password = Void no_hashed_password: a_temp_user.hashed_password = Void
do do
reset_error reset_error
if if
attached a_user.password as l_password and then attached a_temp_user.password as l_password and then
attached a_user.email as l_email attached a_temp_user.email as l_email
then then
storage.new_temp_user (a_user) storage.new_temp_user (a_temp_user)
error_handler.append (storage.error_handler) error_handler.append (storage.error_handler)
else else
error_handler.add_custom_error (0, "bad new user request", "Missing password or email to create new user!") error_handler.add_custom_error (0, "bad new user request", "Missing password or email to create new user!")
@@ -498,13 +497,13 @@ feature -- Change Temp User
storage.remove_activation (a_token) storage.remove_activation (a_token)
end end
delete_temp_user (a_user: CMS_TEMP_USER) delete_temp_user (a_temp_user: CMS_TEMP_USER)
-- Delete user `a_user'. -- Delete user `a_temp_user'.
require require
has_id: a_user.has_id has_id: a_temp_user.has_id
do do
reset_error reset_error
storage.delete_temp_user (a_user) storage.delete_temp_user (a_temp_user)
error_handler.append (storage.error_handler) error_handler.append (storage.error_handler)
end end