Repository navigation
Replies: 6 comments 10 replies
|
Has post_logout_redirect_uris has enabled as a client option now? |
|
Yes. You can send them in dyamic client registration request. |
|
Apart from registering post logout redirect URIs, make sure |
|
Yes, good point! |
|
I've discussed this with @stianst . There is also an option that "Post Logout Redirect URI" will be possible to use "+" as a value, which would indicate that they will be same as Redirect URIs. This is the same approach, which we use already for "Web Origins", which are also inherited from "Redirect URIs" if the value "+" is used. The migration will require adding "+" as a value to all existing clients. It should be possible to do (assuming that we don't want to change DB schema and hence use client attributes for store post logout URI) with some "Insert with select" SQL query, which would do bulk update for all existing clients, which have at least one redirect URI configured on them: |
|
I don't really understand this idea. In the DB, you would store for a client |
|
The SQL query is supposed to be executed only once during first startup of the server with new DB. It is supposed to do "bulk update" and update all existing clients of all realms and add "post.logout.redirect.uris" with value "+" for all clients, which already have at least one redirect URI configured. So after migration, all clients with at least one "Redirect URI" will have "+" as value of "Post Logout Redirect URI" . Hence behaviour for them will be same as before (Redirect URI used for "Post Logout Redirect URI"). However administrator has a chance to remove "+" and configure different "Post Logout Redirect URI" for these clients. Is it more clear now? |
|
Yup got it. Would you keep this feature afterwards? Like the default for new clients would always be to have a "+" there and also the ability for an admin to go back to having a "+" there? |
|
+1 to supporting the spec semantics but with an option to reuse the same URIs for both. I don't necessarily see it as strictly a backwards compatibility feature (though it is that of course), but it's also a convenience that you might still use for new clients. You may even want to default that to be the same (unless there is some security concern with doing so), because it seems to be the exceptional case that they are different. |
|
@alechenninger This is good point. I agree it will probably makes sense to have this switch enabled by default, so "Redirect URI" option refers to both "Login Redirect URI" and "Post Logout Redirect URI" . And just have separate set of "Logout URIs" on demand. This will be automatically ON when migrating and hence we don't need to care about backwards compatibility with old clients. One side-note: I am not 100% sure what are the good practice for typical OIDC generic client adapters and whether they expect different set of redirect URI for "login" and for "logout" . The Keycloak adapters itself did not have any concept of "Login callback" URI and also not a "Logout callback" URI. So for clients secured by Keycloak adapters, it was natural to have only "Redirect URI" used for both login and logout. However some other OIDC client adapters may have the concept of the "login callback" redirect URI and hence it will be more appropriate for them to use different set of redirect URI for login and logout. For JS applications (with "Access Type" public), it likely makes sense to have same "Redirect URI" shared for login and logout. |
|
I've updated the discussion to reflect this. |
|
I also support separating the two redirect URIs. I am wondering what is actually the use case for using the same URL for redirect_uri and post_logout_redirect_uri? In all scenarios I have seen, the two uris are in fact different. They also do something different: the first expects an authcode to get a token, the latter one probably just displays some welcome or goodbye page. So unless you are using some kind of wildcard uri to catch both (which isn't good for security reasons anyways), I don't see a reason to not separate them completely. In my opinion, they should really be different attributes also in the data model. With the new data model in mind, it is probably best to add post_logout_redirect_uris as generic client_attributes. Which would mean to add a data migration to copy these uris. Please note that post_logout_redirect_uris also need to be queried efficiently since on logout, there is a check if there is a matching uri in the DB for any client since client_id might not be available on logout (with no id_token_hint). |
Yes, true. On the other hand, there are probably use-cases when they are same. For example single-page JS applications usually has same redirect URI. Historically, the Keycloak server adapters were also fine with same URI like "http://host/myapp" as the adapter just looked at the presence of query parameters like But having URLs separated is definitely safer and more appropriate for many use-cases.
+1 . We aim to use client attributes and hence not change schema. Regarding migration, my current thinking is towards to use add '+' so existing clients will effectively be configured to use same URIs, which were used as "Redirect URI" .
Yes, however this fact itself (requirement to check all clients) is a bug for both performance and security perspective and we plan to fix it relatively soon. Related issue is here: #8762 |
|
@bs-matil Thanks for the feedback and suggestion.
Yes, that is also one alternative. However the potential disadvantage is, that it will need to introduce changes in the DB schema, which I am not 100% sure is acceptable due the work on the new store. As originally I was wondering if Regarding global switch: I am not sure about this option as it is harder to do it in a way that migrated clients would have backwards compatibility enabled by default, but newly created clients not . The DB migration might be the best, however not sure if it is possible... First question is whether to introduce the support for |
|
I share your concerns. IMHO it looks like the client types are the only reasonable way out without messing up the migration story :/ |
|
Sound good. Regards the migration: |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Current behaviour
Keycloak OIDC client can have set of "Redirect URIs" configured in the admin console. These redirect uris are currently used as valid redirect uris for:
Keycloak currently allows to use any client configured "redirect URI" for both purpose (login callback and logout).
The OIDC specification differs between those 2 cases and it introduces separate client options used in dynamic client registration for both cases:
redirect_urisas client option for login callback: https://openid.net/specs/openid-connect-registration-1_0.html#ClientMetadatapost_logout_redirect_urisas client option used for logout: https://openid.net/specs/openid-connect-rpinitiated-1_0.html#ClientMetadata .New behaviour?
Do we want to make sure that Keycloak is aligned with OIDC specs and introduce separate option for
post_logout_redirect_uris?Advantages of separate options
post_logout_redirect_uris(Clients need to be manually updated during the test, which is a bit hacky)Advantages of current approach with single option
How to address
My vote is to align with OIDC and use separate URIs. However we can have a switch like "Use separate logout redirect URI", which needs to be enabled to use separate redirect URLs for login and logout. This will also address backwards compatibility as existing clients will behave the same. The new clients might have an option to use separate redirect URI for login and logout if they want.
This can be useful for server-side clients, which use the concept of "login callback" redirect URI and similar "logout callback" redirect URI. The fact that they will be separate will make things clearer for those clients. For clients secured by Keycloak adapter (Deprecated right now), this option was not needed as Keycloak adapters did not have concept for "callback" URL (like for example OIDC certification testsuite clients have).
All reactions