Skip to content

Authorization JDBC column limits too small #557

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

Closed
mikesaurus opened this issue Dec 28, 2021 · 3 comments
Closed

Authorization JDBC column limits too small #557

mikesaurus opened this issue Dec 28, 2021 · 3 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@mikesaurus
Copy link

mikesaurus commented Dec 28, 2021

Describe the bug
The oauth2_authorization.attributes, .access_token_metadata, and .oidc_token_metadata database columns are limited to 4000, 2000, and 2000 characters respectively. When introducing a JWT customizer, the token claims set can increase the number of characters required to be stored in the two metadata columns above 2000 characters. Also, the attributes columns includes the authorization request object, which contains "additional parameters" from the authorization request that could consume 2000+ chars by themselves. Any of these situations could result in an error such as:

org.springframework.dao.DataIntegrityViolationException: PreparedStatementCallback; SQL [UPDATE oauth2_authorization SET registered_client_id = ?, principal_name = ?, authorization_grant_type = ?, attributes = ?, state = ?, authorization_code_value = ?, authorization_code_issued_at = ?, authorization_code_expires_at = ?, authorization_code_metadata = ?, access_token_value = ?, access_token_issued_at = ?, access_token_expires_at = ?, access_token_metadata = ?, access_token_type = ?, access_token_scopes = ?, oidc_id_token_value = ?, oidc_id_token_issued_at = ?, oidc_id_token_expires_at = ?, oidc_id_token_metadata = ?, refresh_token_value = ?, refresh_token_issued_at = ?, refresh_token_expires_at = ?, refresh_token_metadata = ? WHERE id = ?]; Value too long for column "ACCESS_TOKEN_METADATA VARCHAR(2000)": "'{""@class"":""java.util.Collections$UnmodifiableMap"",""metadata.token.claims"":{""@class"":""java.util.Collections$UnmodifiableMap"",""su... (2119)"

To Reproduce
The simplest method would be to add a JwtCustomizer to the default implementation that adds a single custom claim with a value containing >2000 chars to the JWT context claims.

Expected behavior
Any database columns that store data that could either be customized (JWT/token metadata) or provided by an external party such as the client and/or user should not make any assumption as to the number of characters that the value may contain and should not be defined as VARCHAR data types with explicit limits, but rather should be defined as CLOB data types. This includes the three columns mentioned above and any others that might fall under this same scenario of unknown/undefined value length.

Sample
@Bean public OAuth2TokenCustomizer<JwtEncodingContext> oAuth2TokenCustomizer() { return (context) -> { final char[] claimValue = new char[2000]; for (int i = 0; i < 2000; i++) { claimValue[i] = 'a'; } context.getClaims().claim("custom_claim", claimValue); }; }

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 4, 2022
@jgrandja jgrandja added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 6, 2022
@sjohnr
Copy link
Contributor

sjohnr commented Jan 7, 2022

Thanks for the report, @mikesaurus. Unfortunately, the JDBC implementation and specifically the corresponding schema are not necessarily able to provide a successful out of the box experience for some. We're aware of this issue, and are prioritizing a number of efforts to mitigate these issues for our community. We believe that when these efforts come together in the next few releases, things will be in a much better state.

As mentioned in this comment:

It is very difficult to provide an implementation that works out of the box for all databases. This implementation strives to use standard sql datatypes and is a simplified JDBC implementation. However, it is designed to be customizable so user's can provide customizations for database vendors that deviate from the standard sql types.

Note: In your case, this comment would also apply to issues with size of columns, etc.

You are free to change the size of columns to match your needs, or even consider changing column types. However, if you are unable to have success simply changing the database schema, take a look at JdbcOAuth2AuthorizationServiceTests.tableDefinitionWhenCustomThenAbleToOverride(), which provides a test on how to override the table definition altogether. There are tests in that class that demonstrate how to customize further.

You may also be interested in this gist which implements a JpaOAuth2AuthorizationService. Also, if you would like to see a guide for implementing the core services with JPA, please up-vote #545, as we're prioritizing what guides to work on for our reference documentation based on community feedback. Lastly, see #558 for ongoing discussions for supporting Redis and MongoDB among other NoSQL vendors. Note that implementations for those data stores will not be provided directly by this project, but community members like you may be interested in contributing them separately.

I'm going to close this for now, but if you are still having issues after you have tried out the proposed solutions we can re-open and discuss further.

@sjohnr sjohnr closed this as completed Jan 7, 2022
@sjohnr sjohnr added status: declined A suggestion or change that we don't feel we should currently apply and removed type: bug A general bug labels Jan 7, 2022
@mikesaurus
Copy link
Author

Hey @sjohnr,
I did see some of the other items that you have out there related to the database and read the comments associated with them. So, I understand what you're currently dealing with. I was easily able to work around the issue by customizing the provided SQL scripts, but I just wanted to report this specific problem since the code provides hooks for customization (which is fantastic), but the ability to customize is limited by the provided data store definition. I imagine that this is a problem that will be best solved through documentation though.

I did see your JPA implementation when we were talking about the NoSQL/Redis support. Thank you for that and for pointing to the JDBC test behavior. Both are super helpful!

@sjohnr
Copy link
Contributor

sjohnr commented Jan 7, 2022

Thanks @mikesaurus. I was on auto-pilot a bit there finding references for you. You obviously knew about that one. 😅

We are definitely working on improving this situation, and are pretty well aware of the issues like the one you mentioned. But of course thanks for engaging in the project, we can't do this without people like you in the community!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

4 participants