From 9d7d43073de2eaf3cdb565d93777dc03c138dc90 Mon Sep 17 00:00:00 2001 From: Jocelyn Fiat Date: Tue, 19 Sep 2017 21:21:30 +0200 Subject: [PATCH] Moved activation implementation into authentication api. Improved core webapi, added registration link, support redirection. Use error webapi response, rather than `send_...` routines. --- modules/auth/cms_authentication_api.e | 43 ++++++++++++++++- modules/auth/cms_authentication_module.e | 37 ++------------- .../auth/cms_user_register_webapi_handler.e | 18 +++---- src/modules/core/cms_user_api.e | 2 +- .../persistence/user/cms_user_storage_i.e | 2 +- .../persistence/user/cms_user_storage_null.e | 2 +- .../persistence/user/cms_user_storage_sql_i.e | 2 +- .../webapi/cms_access_token_webapi_handler.e | 30 ++++++------ .../core/webapi/cms_root_webapi_handler.e | 4 +- .../core/webapi/cms_user_webapi_handler.e | 46 +++--------------- .../core/webapi/cms_users_webapi_handler.e | 16 +++---- src/service/response/json_webapi_response.e | 5 ++ src/service/webapi/cms_webapi_handler.e | 47 ++++++++----------- 13 files changed, 117 insertions(+), 137 deletions(-) diff --git a/modules/auth/cms_authentication_api.e b/modules/auth/cms_authentication_api.e index 54adcb1..c617d18 100644 --- a/modules/auth/cms_authentication_api.e +++ b/modules/auth/cms_authentication_api.e @@ -28,7 +28,7 @@ feature -- Token Generation -- Create activation token l_token := new_token l_user_api.new_activation (l_token, u.id) - l_url_activate := cms_api.absolute_url ("/account/activate/" + l_token, void) + l_url_activate := cms_api.absolute_url ("/account/activate/" + l_token, Void) l_url_reject := cms_api.absolute_url ("/account/reject/" + l_token, Void) -- Send Email to webmaster cms_api.log_debug ("registration", "send_register_email", Void) @@ -43,6 +43,47 @@ feature -- Token Generation cms_api.log ("registration", {STRING_32} "new user %"" + u.name + "%" <" + a_email + ">", {CMS_LOG}.level_info, Void) end + activate_user (a_temp_user: CMS_TEMP_USER; a_token: READABLE_STRING_GENERAL) + require + a_temp_user.has_id + not a_temp_user.is_active + local + l_user_api: CMS_USER_API + l_temp_id: INTEGER_64 + es: CMS_AUTHENTICATION_EMAIL_SERVICE + do + l_temp_id := a_temp_user.id + + -- Valid user_id + a_temp_user.set_id (0) + a_temp_user.mark_active + l_user_api := cms_api.user_api + l_user_api.new_user_from_temp_user (a_temp_user) + + if + not l_user_api.has_error and then + attached l_user_api.user_by_name (a_temp_user.name) as l_new_user + then + if attached a_temp_user.personal_information as l_perso_info then + -- Keep personal information in profile item! + l_user_api.save_user_profile_item (l_new_user, "personal_information", l_perso_info) + end + -- Delete temporal User + a_temp_user.set_id (l_temp_id) + l_user_api.delete_temp_user (a_temp_user) + l_user_api.remove_activation (a_token) + + -- Send Email + if attached l_new_user.email as l_email then + cms_api.log_debug ("activation", "send_contact_activation_confirmation_email", Void) + create es.make (create {CMS_AUTHENTICATION_EMAIL_SERVICE_PARAMETERS}.make (cms_api)) + es.send_contact_activation_confirmation_email (l_email, l_new_user, cms_api.site_url) + end + else + error_handler.add_custom_error (-1, "activation error", "Activation failed!") + end + end + new_token: STRING -- Generate a new token activation token local diff --git a/modules/auth/cms_authentication_module.e b/modules/auth/cms_authentication_module.e index 1d87126..f26b602 100644 --- a/modules/auth/cms_authentication_module.e +++ b/modules/auth/cms_authentication_module.e @@ -520,48 +520,18 @@ feature -- Handler local r: CMS_RESPONSE l_user_api: CMS_USER_API - l_ir: INTERNAL_SERVER_ERROR_CMS_RESPONSE - es: CMS_AUTHENTICATION_EMAIL_SERVICE - l_temp_id: INTEGER_64 do if a_auth_api.cms_api.has_permission ("account activate") then l_user_api := a_auth_api.cms_api.user_api 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_temp_user then - - -- TODO copy the personal information - --! to CMS_USER_PROFILE and persist data - --! check also CMS_USER.data_items - - l_temp_id := l_temp_user.id - - -- 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, a_auth_api.cms_api) + a_auth_api.activate_user (l_temp_user, l_token.value) if - not l_user_api.has_error and then + not a_auth_api.has_error and then attached l_user_api.user_by_name (l_temp_user.name) as l_new_user then - if attached l_temp_user.personal_information as l_perso_info then - -- Keep personal information in profile item! - a_auth_api.cms_api.user_api.save_user_profile_item (l_new_user, "personal_information", l_perso_info) - end - -- Delete temporal 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) - r.set_main_content ("

The account " + a_auth_api.cms_api.user_html_link (l_new_user) + " has been activated

") - -- Send Email - if attached l_new_user.email as l_email then - create es.make (create {CMS_AUTHENTICATION_EMAIL_SERVICE_PARAMETERS}.make (a_auth_api.cms_api)) - write_debug_log (generator + ".handle register: send_contact_activation_confirmation_email") - es.send_contact_activation_confirmation_email (l_email, l_new_user, req.absolute_script_url ("")) - end else -- Failure!!! r.set_status_code ({HTTP_CONSTANTS}.internal_server_error) @@ -578,8 +548,7 @@ feature -- Handler end r.execute else - create l_ir.make (req, res, a_auth_api.cms_api) - l_ir.execute + (create {INTERNAL_SERVER_ERROR_CMS_RESPONSE}.make (req, res, a_auth_api.cms_api)).execute end else a_auth_api.cms_api.response_api.send_access_denied (Void, req, res) diff --git a/modules/auth/cms_user_register_webapi_handler.e b/modules/auth/cms_user_register_webapi_handler.e index e0683be..09986ab 100644 --- a/modules/auth/cms_user_register_webapi_handler.e +++ b/modules/auth/cms_user_register_webapi_handler.e @@ -32,14 +32,14 @@ feature -- Execution if req.is_post_request_method then register_user (req, res) else - send_bad_request (Void, req, res) + new_bad_request_error_response (Void, req, res).execute end end register_user (req: WSF_REQUEST; res: WSF_RESPONSE) local f: CMS_FORM - rep: like new_webapi_response + rep: like new_response l_user_api: CMS_USER_API u: CMS_TEMP_USER l_exist: BOOLEAN @@ -60,7 +60,7 @@ feature -- Execution f.extend_text_field ("email", Void) f.extend_text_field ("personal_information", Void) - rep := new_webapi_response (req, res) + rep := new_response (req, res) f.process (rep) if attached f.last_data as fd and then not fd.has_error and then @@ -83,7 +83,7 @@ feature -- Execution l_exist := True end if fd.has_error or l_exist then - send_bad_request ("User name or email is already taken!", req, res) + rep := new_bad_request_error_response ("User name or email is already taken!", req, res) else -- New temp user create u.make (l_name) @@ -92,20 +92,22 @@ feature -- Execution u.set_personal_information (l_personal_information) auth_api.register_user (u, l_email, l_personal_information) + -- Until it is activated, this is not a real user. -- add_user_links_to (u, rep) rep.add_string_field ("status", "succeed") + rep.add_string_field ("information", "Waiting for activation") rep.add_self (req.percent_encoded_path_info) - rep.execute end else - send_bad_request ("Invalid email", req, res) + rep := new_access_denied_error_response ("Invalid email", req, res) end else - send_bad_request ("There were issue with your application, invalid or missing values.", req, res) + rep := new_access_denied_error_response ("There were issue with your application, invalid or missing values.", req, res) end else - send_access_denied ("You can also contact the webmaster to ask for an account.", req, res) + rep := new_access_denied_error_response ("You can also contact the webmaster to ask for an account.", req, res) end + rep.execute end diff --git a/src/modules/core/cms_user_api.e b/src/modules/core/cms_user_api.e index eab6638..1b99c57 100644 --- a/src/modules/core/cms_user_api.e +++ b/src/modules/core/cms_user_api.e @@ -575,7 +575,7 @@ feature -- Change Temp User end end - remove_activation (a_token: READABLE_STRING_32) + remove_activation (a_token: READABLE_STRING_GENERAL) -- Remove activation token `a_token', from the user_storage. do user_storage.remove_activation (a_token) diff --git a/src/modules/core/persistence/user/cms_user_storage_i.e b/src/modules/core/persistence/user/cms_user_storage_i.e index 35914fe..ce38b2f 100644 --- a/src/modules/core/persistence/user/cms_user_storage_i.e +++ b/src/modules/core/persistence/user/cms_user_storage_i.e @@ -265,7 +265,7 @@ feature -- New Temp User deferred end - remove_activation (a_token: READABLE_STRING_32) + remove_activation (a_token: READABLE_STRING_GENERAL) -- Remove activation by token `a_token'. deferred end diff --git a/src/modules/core/persistence/user/cms_user_storage_null.e b/src/modules/core/persistence/user/cms_user_storage_null.e index 3572485..56a9bc9 100644 --- a/src/modules/core/persistence/user/cms_user_storage_null.e +++ b/src/modules/core/persistence/user/cms_user_storage_null.e @@ -191,7 +191,7 @@ feature -- Temp Users end - remove_activation (a_token: READABLE_STRING_32) + remove_activation (a_token: READABLE_STRING_GENERAL) -- . do end diff --git a/src/modules/core/persistence/user/cms_user_storage_sql_i.e b/src/modules/core/persistence/user/cms_user_storage_sql_i.e index d34e1a8..fd18c86 100644 --- a/src/modules/core/persistence/user/cms_user_storage_sql_i.e +++ b/src/modules/core/persistence/user/cms_user_storage_sql_i.e @@ -1312,7 +1312,7 @@ feature -- New Temp User feature -- Remove Activation - remove_activation (a_token: READABLE_STRING_32) + remove_activation (a_token: READABLE_STRING_GENERAL) -- . local l_parameters: STRING_TABLE [detachable ANY] diff --git a/src/modules/core/webapi/cms_access_token_webapi_handler.e b/src/modules/core/webapi/cms_access_token_webapi_handler.e index 5b8c79c..40d6ea9 100644 --- a/src/modules/core/webapi/cms_access_token_webapi_handler.e +++ b/src/modules/core/webapi/cms_access_token_webapi_handler.e @@ -28,10 +28,10 @@ feature -- Execution elseif req.is_get_request_method then get_access_token (l_uid, req, res) else - send_bad_request (Void, req, res) + new_bad_request_error_response (Void, req, res).execute end else - send_bad_request ("Missing {uid} parameter", req, res) + new_bad_request_error_response ("Missing {uid} parameter", req, res).execute end end @@ -52,28 +52,28 @@ feature -- Request execution if attached user_by_uid (a_uid) as l_user then if attached api.user as u then if u.same_as (l_user) or api.user_api.is_admin_user (u) then - rep := new_access_token_webapi_response (l_user, user_access_token (l_user), req, res) + rep := new_access_token_response (l_user, user_access_token (l_user), req, res) if attached {WSF_STRING} req.item ("destination") as dest then rep.set_redirection (dest.url_encoded_value) end - rep.execute else -- Only admin, or current user can see its access_token! - send_access_denied (Void, req, res) + rep := new_access_denied_error_response (Void, req, res) end else - send_access_denied (Void, req, res) + rep := new_access_denied_error_response (Void, req, res) end else - send_not_found ("User not found", req, res) + rep := new_not_found_error_response ("User not found", req, res) end + rep.execute end post_access_token (a_uid: READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE) -- Execute handler for `req' and respond in `res'. local l_access_token: detachable READABLE_STRING_32 - rep: like new_webapi_response + rep: like new_response do if attached user_by_uid (a_uid) as l_user then if attached api.user as u then @@ -91,21 +91,21 @@ feature -- Request execution -- end set_user_access_token (l_user, l_access_token) - rep := new_access_token_webapi_response (l_user, l_access_token, req, res) + rep := new_access_token_response (l_user, l_access_token, req, res) if attached {WSF_STRING} req.item ("destination") as dest then rep.set_redirection (dest.url_encoded_value) end - rep.execute else -- Only admin, or current user can create the user access_token! - send_access_denied (Void, req, res) + rep := new_access_denied_error_response (Void, req, res) end else - send_access_denied (Void, req, res) + rep := new_access_denied_error_response (Void, req, res) end else - send_not_found ("User not found", req, res) + rep := new_not_found_error_response ("User not found", req, res) end + rep.execute end feature {NONE} -- Implementation @@ -128,11 +128,11 @@ feature {NONE} -- Implementation api.user_api.save_user_profile_item (a_user, "access_token", a_access_token) end - new_access_token_webapi_response (a_user: CMS_USER; a_access_token: detachable READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE): like new_webapi_response + new_access_token_response (a_user: CMS_USER; a_access_token: detachable READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE): like new_response local tb: STRING_TABLE [detachable ANY] do - Result := new_webapi_response (req, res) + Result := new_response (req, res) if a_access_token /= Void then Result.add_string_field ("access_token", a_access_token) else diff --git a/src/modules/core/webapi/cms_root_webapi_handler.e b/src/modules/core/webapi/cms_root_webapi_handler.e index c905a34..97a108b 100644 --- a/src/modules/core/webapi/cms_root_webapi_handler.e +++ b/src/modules/core/webapi/cms_root_webapi_handler.e @@ -21,10 +21,12 @@ feature -- Execution local rep: HM_WEBAPI_RESPONSE do - rep := new_webapi_response (req, res) + rep := new_response (req, res) rep.add_string_field ("site_name", api.setup.site_name) if attached api.user as u then add_user_links_to (u, rep) + elseif api.has_permission ("account register") then + rep.add_link ("register", Void, api.webapi_path ("/account/register")) end rep.add_self (req.percent_encoded_path_info) rep.execute diff --git a/src/modules/core/webapi/cms_user_webapi_handler.e b/src/modules/core/webapi/cms_user_webapi_handler.e index a069a97..d9f5ac5 100644 --- a/src/modules/core/webapi/cms_user_webapi_handler.e +++ b/src/modules/core/webapi/cms_user_webapi_handler.e @@ -24,7 +24,7 @@ feature -- Execution -- elseif req.is_post_request_method then -- execute_post (req, res) else - send_bad_request (Void, req, res) + new_bad_request_error_response (Void, req, res).execute end end @@ -46,7 +46,7 @@ feature -- Execution -- end if l_user /= Void then if l_user.same_as (u) or api.has_permissions (<<"admin users", "view users">>) then - rep := new_webapi_response (req, res) + rep := new_response (req, res) rep.add_string_field ("uid", l_user.id.out) rep.add_string_field ("name", l_user.name) if attached l_user.email as l_email then @@ -66,56 +66,24 @@ feature -- Execution end add_user_links_to (l_user, rep) else - rep := new_wepapi_error_response ("denied", req, res) + rep := new_error_response ("denied", req, res) rep.set_status_code ({HTTP_STATUS_CODE}.user_access_denied) end else - rep := new_wepapi_error_response ("Not found", req, res) + rep := new_error_response ("Not found", req, res) rep.set_status_code ({HTTP_STATUS_CODE}.not_found) end else - rep := new_wepapi_error_response ("Bad request", req, res) + rep := new_error_response ("Bad request", req, res) rep.set_status_code ({HTTP_STATUS_CODE}.bad_request) end - rep.execute else -- FIXME: use specific Web API response! - send_access_denied (Void, req, res) + rep := new_access_denied_error_response (Void, req, res) end + rep.execute end --- execute_post (req: WSF_REQUEST; res: WSF_RESPONSE) --- -- Execute handler for `req' and respond in `res'. --- local --- rep: HM_WEBAPI_RESPONSE --- l_user: detachable CMS_USER --- do --- if attached api.user as u and then api.has_permission ("admin users") then --- if attached {WSF_STRING} req.path_parameter ("uid") as p_uid then --- if p_uid.is_integer then --- l_user := api.user_api.user_by_id (p_uid.integer_value) --- else --- l_user := api.user_api.user_by_name (p_uid.value) --- end ----- if l_user = Void and p_uid.is_case_insensitive_equal ("me") then ----- l_user := u ----- end --- if l_user /= Void then --- else --- rep := new_wepapi_error_response ("Not found", req, res) --- rep.set_status_code ({HTTP_STATUS_CODE}.not_found) --- end --- else --- rep := new_wepapi_error_response ("Bad request", req, res) --- rep.set_status_code ({HTTP_STATUS_CODE}.bad_request) --- end --- rep.execute --- else --- -- FIXME: use specific Web API response! --- send_access_denied (Void, req, res) --- end --- end - note copyright: "2011-2017, Jocelyn Fiat, Javier Velilla, Eiffel Software and others" diff --git a/src/modules/core/webapi/cms_users_webapi_handler.e b/src/modules/core/webapi/cms_users_webapi_handler.e index fef81f4..0e6558c 100644 --- a/src/modules/core/webapi/cms_users_webapi_handler.e +++ b/src/modules/core/webapi/cms_users_webapi_handler.e @@ -24,7 +24,7 @@ feature -- Execution elseif req.is_post_request_method then execute_post (req, res) else - send_bad_request (Void, req, res) + new_bad_request_error_response (Void, req, res).execute end end @@ -43,7 +43,7 @@ feature -- Execution if attached req.query_parameter ("full") as p and then p.is_case_insensitive_equal ("yes") then l_full := True end - rep := new_webapi_response (req, res) + rep := new_response (req, res) nb := api.user_api.users_count rep.add_integer_64_field ("users_count", nb) create l_params.make (0, nb.to_natural_32) @@ -77,10 +77,10 @@ feature -- Execution end rep.add_iterator_field ("users", arr) rep.add_self (req.percent_encoded_path_info) - rep.execute else - send_access_denied (Void, req, res) + rep := new_access_denied_error_response (Void, req, res) end + rep.execute end execute_post (req: WSF_REQUEST; res: WSF_RESPONSE) @@ -155,17 +155,17 @@ feature -- Execution end end if l_user = Void or else err /= Void then - rep := new_wepapi_error_response (err, req, res) + rep := new_error_response (err, req, res) else - rep := new_webapi_response (req, res) + rep := new_response (req, res) rep.add_string_field ("uid", l_user.id.out) add_user_links_to (l_user, rep) end rep.add_self (req.percent_encoded_path_info) - rep.execute else - send_access_denied (Void, req, res) + rep := new_access_denied_error_response (Void, req, res) end + rep.execute end diff --git a/src/service/response/json_webapi_response.e b/src/service/response/json_webapi_response.e index d2828d6..0ff4050 100644 --- a/src/service/response/json_webapi_response.e +++ b/src/service/response/json_webapi_response.e @@ -127,6 +127,11 @@ feature -- Execution m: WSF_PAGE_RESPONSE do create m.make_with_body (resource.representation) + m.set_status_code (status_code) + if attached redirection as loc then + m.header.put_location (loc) + m.set_status_code ({HTTP_STATUS_CODE}.temp_redirect) + end m.header.put_content_type ("application/json") response.send (m) end diff --git a/src/service/webapi/cms_webapi_handler.e b/src/service/webapi/cms_webapi_handler.e index 6d9a4b5..d9f6746 100644 --- a/src/service/webapi/cms_webapi_handler.e +++ b/src/service/webapi/cms_webapi_handler.e @@ -32,59 +32,52 @@ feature -- API Service feature -- Factory - new_webapi_response (req: WSF_REQUEST; res: WSF_RESPONSE): HM_WEBAPI_RESPONSE + new_response (req: WSF_REQUEST; res: WSF_RESPONSE): HM_WEBAPI_RESPONSE do -- create {MD_WEBAPI_RESPONSE} Result.make (req, res, api) create {JSON_WEBAPI_RESPONSE} Result.make (req, res, api) end - new_wepapi_error_response (msg: detachable READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE): HM_WEBAPI_RESPONSE + new_error_response (msg: detachable READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE): like new_response do - Result := new_webapi_response (req, res) + Result := new_response (req, res) + Result.set_status_code ({HTTP_STATUS_CODE}.bad_request) if msg /= Void then Result.add_string_field ("error", msg) else Result.add_string_field ("error", "True") end + Result.add_self (req.request_uri) end - send_not_found (m: detachable READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE) - local - rep: HM_WEBAPI_RESPONSE + new_not_found_error_response (m: detachable READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE): like new_response do - if m /= Void then - rep := new_wepapi_error_response (m, req, res) + if m = Void then + Result := new_error_response ("Not Found", req, res) else - rep := new_wepapi_error_response ("Not found", req, res) + Result := new_error_response (m, req, res) end - rep.set_status_code ({HTTP_STATUS_CODE}.not_found) - rep.execute + Result.set_status_code ({HTTP_STATUS_CODE}.not_found) end - send_access_denied (m: detachable READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE) - local - rep: HM_WEBAPI_RESPONSE + new_access_denied_error_response (m: detachable READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE): like new_response do - if m /= Void then - rep := new_wepapi_error_response (m, req, res) + if m = Void then + Result := new_error_response ("Access denied", req, res) else - rep := new_wepapi_error_response ("Access denied", req, res) + Result := new_error_response (m, req, res) end - rep.set_status_code ({HTTP_STATUS_CODE}.user_access_denied) - rep.execute + Result.set_status_code ({HTTP_STATUS_CODE}.user_access_denied) end - send_bad_request (m: detachable READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE) - local - rep: HM_WEBAPI_RESPONSE + new_bad_request_error_response (m: detachable READABLE_STRING_GENERAL; req: WSF_REQUEST; res: WSF_RESPONSE): like new_response do - if m /= Void then - rep := new_wepapi_error_response (m, req, res) + if m = Void then + Result := new_error_response ("Bad request", req, res) else - rep := new_wepapi_error_response ("Bad request", req, res) + Result := new_error_response (m, req, res) end - rep.set_status_code ({HTTP_STATUS_CODE}.bad_request) - rep.execute + Result.set_status_code ({HTTP_STATUS_CODE}.bad_request) end feature {NONE} -- Builder