-
Notifications
You must be signed in to change notification settings - Fork 8.7k
#50798 Apply protocol defaults when creating clients through Admin AP… #50801
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.
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
Changes from all commits
764bead
f8340c5
57e3ab4
5a68501
59488c5
ebe3bcf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,8 +31,11 @@ | |
| import org.keycloak.models.mapper.ClientModelMappers; | ||
| import org.keycloak.models.utils.KeycloakModelUtils; | ||
| import org.keycloak.models.utils.ModelToRepresentation; | ||
| import org.keycloak.protocol.LoginProtocol; | ||
| import org.keycloak.protocol.LoginProtocolFactory; | ||
| import org.keycloak.representations.admin.v2.BaseClientRepresentation; | ||
| import org.keycloak.representations.admin.v2.OIDCClientRepresentation; | ||
| import org.keycloak.representations.admin.v2.SAMLClientRepresentation; | ||
| import org.keycloak.representations.admin.v2.validation.CreateClient; | ||
| import org.keycloak.representations.admin.v2.validation.PatchClient; | ||
| import org.keycloak.representations.admin.v2.validation.PutClient; | ||
|
|
@@ -339,6 +342,10 @@ private CreateOrUpdateResult createOrUpdate(RealmModel realm, String clientId, B | |
| } | ||
| validator.validate(client, strategy.getValidationGroup(), Default.class); | ||
| var proposedRepresentation = getProposedOldRepresentation(realm, client, mapper); | ||
| if (client instanceof SAMLClientRepresentation samlClient) { | ||
| proposedRepresentation.setStandardFlowEnabled(null); | ||
| proposedRepresentation.setFrontchannelLogout(samlClient.getFrontChannelLogout()); | ||
| } | ||
| session.clientPolicy().triggerOnEvent(new AdminClientRegisterContext(proposedRepresentation, permissions.adminAuth())); | ||
|
|
||
| // Add basic attributes | ||
|
|
@@ -348,6 +355,7 @@ private CreateOrUpdateResult createOrUpdate(RealmModel realm, String clientId, B | |
| // Generate random secret if applicable | ||
| generateClientSecretIfNeeded(client, model, strategy, patchExplicitNullSecret); | ||
| mapper.toModel(client, model); | ||
| setupClientDefaults(client, model, proposedRepresentation); | ||
|
|
||
| // Validate the fully populated model | ||
| ValidationUtil.validateClient(session, model, true, r -> { | ||
|
|
@@ -373,6 +381,22 @@ private CreateOrUpdateResult createOrUpdate(RealmModel realm, String clientId, B | |
| return new CreateOrUpdateResult(mapper.fromModel(model), !alreadyExists); | ||
| } | ||
|
|
||
| private void setupClientDefaults(BaseClientRepresentation client, ClientModel model, ClientRepresentation proposedRepresentation) { | ||
| LoginProtocolFactory factory = (LoginProtocolFactory) session.getKeycloakSessionFactory() | ||
| .getProviderFactory(LoginProtocol.class, client.getProtocol()); | ||
| if (factory != null) { | ||
| factory.setupClientDefaults(proposedRepresentation, model); | ||
| } | ||
| if (client instanceof OIDCClientRepresentation oidcClient | ||
| && oidcClient.getAuth() != null | ||
| && !isClientSecret(oidcClient.getAuth().getMethod()) | ||
| && isBlank(oidcClient.getAuth().getSecret())) { | ||
| // OIDCLoginProtocolFactory generates a secret for every confidential client, while Admin API v2 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but you do this even when
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm ok with leaving it as is, but it's certainly fine to move this block into the preceding if (factory != null) block.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, not a bug, I I wasn't certain that it can't have side effects which brought me to reading the comment. At least the comment is wrong, but I am fine with it as is. |
||
| // only uses secrets with secret-based authentication methods. | ||
| model.setSecret(null); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Fires a v2 admin event for client operations (only enabled for testing now to avoid duplicated admin events) | ||
| * | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.