KEYCLOAK-2359 - Expose current clientId in email templates.

[thomasdarimont]

We hereby expose the current clientId in email templates in order to be able
to generate client application specific links.
If a clientId is not available we fall back to the "account" clientId.

[thomasdarimont]

The usage of the clientId ({3}) is a test - suggestions for a more sane default text that uses the client id or should this be just omitted from the messages.properties for now?

[stianst]

Thinking some more about this I was thinking that we probably want to make sure the client-id is always available. It shouldn't require manually including in the email. I think we should add it to the links by default and include in the signature part as well to prevent forgery.

[thomasdarimont]

Having the clientId accessible in general would make a lot of things easier - shall I close this PR or could you give some hints about how to rework it?

[stianst]

I'm not sure if this is two separate issues or not, but I think the client base-url should always be available in the login flows. Even when the session has timed out. This would make it always possible to have a return to the application link on any page from the login flows. I think the best way to do this is to add the uuid of the client to the ClientSessionCode. It will make links slightly longer, but would improve usability.

Would that cover what you want?

[stianst]

Closing this as I didn't get any feedback. Let's talk about this on the mailing list.

[thomasdarimont]

Okay, sorry for not giving feedback in time - I'm fine with closing this issue - as I wasn't sure how to proceed - adding the clientId (uuid) to the ClientSessionCode sounded like it should happen in the context of a new dedicated ticket - though it would be related to this one.

Looking at ClientSessionCode - I notice that one can already access the client id via clientSessionCode .getClientSession().getClient().getId()/getClientId() - is this already available in emails somehow?
if not, how would the clientId then accessed from an email template if the clientId would be available in ClientSessionCode?

[stianst]

You can get the client from the client session, but only as long as the client session hasn't been garbage collected. If we added the client uuid directly to the client session code it would always be available.

[thomasdarimont]

Okay, I understand that. So this all would then boil down to adding the clientId to the access code AND extracting the clientId when the code is parsed again, right? *)
if so, should the added clientId be used always or only as a fallback if the clientSession isn't available anymore?

Sorry for continuing here - but I think we're almost there ;-)

*) currently ClientSessionCode parses the code string back into a ClientSessionCode instance. How about adding a flag (dead, alive) to ClientSessionCodethat indicates the ClientSession state. dead if keycloakSession.sessions().getClientSession(id) yields null otherwise alive.

If alive we could can use the information from the active clientSession. otherwise we can fallback to creating a "zombie" ClientSessionModel that provides access to a ClientModel retrieved via the associated clientId (uuid).

[stianst]

That sounds to complicate to me. All we need is the ability to add the client to the forms if the session is missing so we can add a back to client link. No need for a zombie client session model I'd think.

I need to have a chat to Bill about this, then we'll see if we can do something for 1.9 around this.

[thomasdarimont]

Okay, thanks :)

[thomasdarimont]

Another idea:

In LoginActionsService.verifyCode(..)

keycloak/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java

Line 219 in 06e44b1

ClientSessionCode.ParseResult result = ClientSessionCode.parseResult(code, session, realm);

the ClientSessionCode.parseResult(..) is used which already deals with the case that the client session is not there anymore.
Perhaps ClientSessionCode.parseResult could add the "requesting" clientId (to be added to the code) to the ParseResult. This could then be used as a fallback in LoginActionsService.verifyCode(..) in this case:

keycloak/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java

Line 226 in 06e44b1

event.clone().detail(Details.RESTART_AFTER_TIMEOUT, "true").error(Errors.EXPIRED_CODE);

This would enable keycloak to always redirect a user to the originally request client which would then retrigger the proper authentication flow.

[patriot1burke]

Stian made a big deal about the size of our URLs a year or so ago.

[stianst]

I don't like increasing the size of the client session code as it increases the length of URLs. However, I think we need to make sure the client is always known so we can have a back to application link on all error pages. I can't think of any other way to do it than to add the client uuid to the client session code as well.