SSO OIDC: Fix default issuer and endpoint URLs - #2101
Merged
Conversation
Derive the default OIDC issuer from the complete SSO route instead of falling back to the Admidio base URL. This restores the missing /oidc path in discovery, authorization, token, UserInfo and JWKS URLs. When the default value is not changed, instead of storing the default value in the database, store an empty string and let the OIDCService class interpret empty issuer URLs as default value. When an installation is moved to a different (sub)domain, the admidio OIDC-part will adjust accordingly. In the OIDC settings, display the resolved issuer in the SSO settings as a placeholder text if the input field is empty, but avoid persisting the default so that later changes to the Admidio base URL are reflected automatically. Preserve explicitly configured issuer URLs and normalize trailing slashes. This is only an issue for OIDC (where actual URLs are configured), and must NOT be applied for SAML (where the entity ID is really a unique identifier and not a URL). To show the placeholder text and also copy it with the copy icon next to the input field, the JS handler is extended accordingly. This functionality can be used in any input field by simply adding the 'placeholder' property when the 'copy-container' css class is assigned. Fixes Admidio#2078 and Admidio#2079
Member
|
Does this also fix a security advisory of the SSO module? |
Contributor
Author
|
No, unfortunately not. This patch is only about the settings and which values are stored in the DB and which are derived from the current host name. This patch does not change any of the security-related code parts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Derive the default OIDC issuer from the complete SSO route instead of falling back to the Admidio base URL. This restores the missing /oidc path in discovery, authorization, token, UserInfo and JWKS URLs.
When the default value is not changed, instead of storing the default value in the database, store an empty string and let the OIDCService class interpret empty issuer URLs as default value. When an installation is moved to a different (sub)domain, the admidio OIDC-part will adjust accordingly.
In the OIDC settings, display the resolved issuer in the SSO settings as a placeholder text if the input field is empty, but avoid persisting the default so that later changes to the Admidio base URL are reflected automatically.
Preserve explicitly configured issuer URLs and normalize trailing slashes. This is only an issue for OIDC (where actual URLs are configured), and must NOT be applied for SAML (where the entity ID is really a unique identifier and not a URL).
To show the placeholder text and also copy it with the copy icon next to the input field, the JS handler is extended accordingly. This functionality can be used in any input field by simply adding the 'placeholder' property when the 'copy-container' css class is assigned.
Fixes #2078 and #2079