8000 Use email verification instead of executing action for `send-verify-email` endpoint by lexcao · Pull Request #23303 · keycloak/keycloak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use email verification instead of executing action for send-verify-email endpoint #23303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 11, 2024
Merged

Use email verification instead of executing action for send-verify-email endpoint #23303

merged 1 commit into from
Jan 11, 2024

Conversation

lexcao
Copy link
Contributor
@lexcao lexcao commented Sep 17, 2023

Closes #15190

Add support for send-verify-email endpoint to use the email-verification.ftl instead of executeActions.ftl

Also introduce a new parameter lifespan to be able to override the default lifespan value (12 hours)

@lexcao lexcao requested review from a team as code owners September 17, 2023 16:47
@lexcao lexcao requested a review from a team September 17, 2023 16:47
@lexcao lexcao requested a review from a team as a code owner September 17, 2023 16:47
This was referenced Sep 17, 2023
Closed
Closed
@Iseeumhmm
Copy link

Can we get a bump on this. This will fix an issue we are having

@zyv zyv mentioned this pull request Dec 19, 2023
Closed
@lexcao lexcao force-pushed the feat/send-verify-email branch 2 times, most recently from fe4190a to 506494f Compare January 8, 2024 14:44
@lexcao
Copy link
Contributor Author
lexcao commented Jan 8, 2024
8000

Hi @mposolda or @pedroigor
I have updated the PR, maybe we can bump it up if the CI is all green.

@lexcao
Use email verification instead of executing action for `send-verify-e…
ab041ed 
…mail` endpoint

Closes keycloak#15190

Add support for `send-verify-email` endpoint to use the `email-verification.ftl` instead of `executeActions.ftl`

Also introduce a new parameter `lifespan` to be able to override the default lifespan value (12 hours)

Signed-off-by: Lex Cao <lexcao@foxmail.com>
@lexcao lexcao force-pushed the feat/send-verify-email branch from 506494f to ab041ed Compare January 8, 2024 16:42
This was referenced Jan 8, 2024
Closed
Closed
Closed
pedroigor
pedroigor approved these changes Jan 9, 2024
View reviewed changes
Copy link
Contributor
@pedroigor pedroigor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mposolda LGTM. Wdyt?

mposolda
mposolda reviewed Jan 9, 2024
View reviewed changes
Copy link
Contributor
@mposolda mposolda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lexcao Thanks! I think this change is good, but is it also possible to add some small note to the migration guide (file changes-24_0_0.adoc) as this is behaviour change and it is possible that someone relies on the older behaviour for some reason?

@mposolda mposolda self-assigned this Jan 9, 2024
@yegortokmakov
Copy link

@lexcao, thanks for creating the PR!

quick question: as of now the patch will send out an email with email validation tempalte, but the link still points to actions required. Will it take a lot of work to use the direct email validation link in the email? To avoid extra manual click.

@lexcao
Copy link
Contributor Author
lexcao commented Jan 10, 2024

@mposolda
Sure, I will add it.

@lexcao
Copy link
Contributor Author
lexcao commented Jan 10, 2024

Hi @yegortokmakov
Here are the screenshots from my local I can show you.

This is the email look like.
image

And the link is something like this.

/realms/{realm}/login-actions/action-token?key={token}

Click the email shows:
image

After clicking Click here to proceed, and we're done.
image

@yegortokmakov
Copy link

Hi @yegortokmakov Here are the screenshots from my local I can show you.
Click the email shows: image

This step seems to be unnecessary. Wdyt?

@lexcao
Copy link
Contributor Author
lexcao commented Jan 10, 2024

Hi @yegortokmakov
I am not sure should we change the behavior, or skip that step.
@mposolda or @pedroigor please help check on this

This was handled by the VerifyEmailActionTokenHandler, which is shown when the auth session not found.
There is no auth session when using Admin API.

keycloak/services/src/main/java/org/keycloak/authentication/actiontoken/verifyemail/VerifyEmailActionTokenHandler.java

Lines 88 to 102 in 583d31b

if (tokenContext.isAuthenticationSessionFresh()) {
// Update the authentication session in the token
token.setCompoundOriginalAuthenticationSessionId(token.getCompoundAuthenticationSessionId());
String authSessionEncodedId = AuthenticationSessionCompoundId.fromAuthSession(authSession).getEncodedId();
token.setCompoundAuthenticationSessionId(authSessionEncodedId);
UriBuilder builder = Urls.actionTokenBuilder(uriInfo.getBaseUri(), token.serialize(session, realm, uriInfo),
authSession.getClient().getClientId(), authSession.getTabId());
String confirmUri = builder.build(realm.getName()).toString();
return session.getProvider(LoginFormsProvider.class)
.setAuthenticationSession(authSession)
.setSuccess(Messages.CONFIRM_EMAIL_ADDRESS_VERIFICATION, user.getEmail())
.setAttribute(Constants.TEMPLATE_ATTR_ACTION_URI, confirmUri)
.createInfoPage();

@pedroigor
Copy link
Contributor
pedroigor commented Jan 10, 2024 •
edited
Loading

@yegortokmakov @lexcao Sorry, which one of the steps? I would propose doing any other improvement as a follow-up.

@yegortokmakov
Copy link

@yegortokmakov @lexcao Sorry, which one of the steps? I would propose doing any other improvement as a follow-up.

sure, makes sense! I can open a follow-up issue where we can discuss it

@pedroigor pedroigor merged commit 47f7e3e into keycloak:main Jan 11, 2024
@lexcao lexcao deleted the feat/send-verify-email branch January 12, 2024 01:43
@lexcao
Copy link
Contributor Author
lexcao commented Jan 12, 2024

Hi @pedroigor
Thanks, I will add changes-24_0_0.adoc in the follow-up.

add some small note to the migration guide (file changes-24_0_0.adoc) as this is behaviour change and it is possible that someone relies on the older behaviour for some reason

@mposolda mposolda mentioned this pull request Jan 12, 2024
Closed
@mposolda
Copy link
Contributor

@lexcao Thanks! Created #26146 as a follow-up task for the note. Pr for this would be welcome.

@reda-alaoui reda-alaoui mentioned this pull request Feb 8, 2024
Closed
@yegortokmakov yegortokmakov mentioned this pull request Nov 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mposolda mposolda mposolda left review comments

@cagataygurturk cagataygurturk cagataygurturk approved these changes

@pedroigor pedroigor pedroigor approved these changes

Assignees

@mposolda mposolda

Labels
flaky-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RestAPI endpoint "send-verify-email" sending execute actions email template.
7 participants
@lexcao @Iseeumhmm @yegortokmakov @pedroigor @mposolda @cagataygurturk @ahus1
0