Skip to content

KEYCLOAK-18268 logo_uri, policy_uri and tos_uri ( client attributes)#8082

Closed
cgeorgilakis wants to merge 1 commit intokeycloak:masterfrom
eosc-kc:86-client-enrich
Closed

KEYCLOAK-18268 logo_uri, policy_uri and tos_uri ( client attributes)#8082
cgeorgilakis wants to merge 1 commit intokeycloak:masterfrom
eosc-kc:86-client-enrich

Conversation

@cgeorgilakis
Copy link
Copy Markdown
Contributor

@cgeorgilakis cgeorgilakis commented May 24, 2021

Keycloak Documentation PR : keycloak/keycloak-documentation#1181.
Needed for this PR.

@cgeorgilakis
Copy link
Copy Markdown
Contributor Author

Our goal was to do no database changes. However, for logo_uri some SAML clients use data:image and the logo_uri length is over 4000 characters. Current database length for VALUE field of CLIENT_ATTRIBUTES is insufficient.
We believe that we should create a new field in ClientEntity for logo_uri with text format. Do you agree or you would like to propose a different solution?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be more strict: "Schemes other than “https”, “http”, or “data” SHOULD NOT be used."
according to https://docs.oasis-open.org/security/saml/Post2.0/sstc-saml-metadata-ui/v1.0/os/sstc-saml-metadata-ui-v1.0-os.html#__RefHeading__10407_1021935550

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree that check must be more strict. However, I am not sure that we should folllow SAML documentation for OIDC clients.

I try to use already implemented check for URIs. However, in logo URI "data" scheme must be supported. What Keycloak team believe about Logo URI validation? Maybe we should check if scheme is equal to "data". This means that URI is accepted. Otherwise uri.toURL() must be executed.

@guymoyo
Copy link
Copy Markdown

guymoyo commented Aug 5, 2021

CLIENT_ATTRIBUTES

Recently there were some requirements where REALM_ATTRIBUTE needed to be extended, the same can be applied in CLIENT_ATTRIBUTE.

<dropColumn tableName="REALM_ATTRIBUTE" columnName="VALUE"/> <renameColumn tableName="REALM_ATTRIBUTE" oldColumnName="VALUE_NEW" newColumnName="VALUE" columnDataType="NCLOB"/>

@cgeorgilakis
Copy link
Copy Markdown
Contributor Author

Been merged into this PR in order to be one PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants