Skip to content

Provide a Jackson2Module for OAuth2Authorization #1970

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
OrangeDog opened this issue Apr 14, 2025 · 8 comments
Closed

Provide a Jackson2Module for OAuth2Authorization #1970

OrangeDog opened this issue Apr 14, 2025 · 8 comments
Assignees
Labels
status: duplicate A duplicate of another issue

Comments

@OrangeDog
Copy link

Expected Behavior
All the other Spring Security components provide the necessary config to allow (de)serialization of their Authentication object trees with Jackson out of the box.

https://docs.spring.io/spring-security/site/docs/current/api/org/springframework/security/jackson2/SecurityJackson2Modules.html

Most of this seems to have been done in some of the samples. They could just be moved into the main library and registered with the core lookup.

Current Behavior
Every user must construct their own mappings, mixins, deserializers, and whitelists if they want to do anything with JSON-based storage (e.g. #558).

Context
The other main option of using Java Serialization is blocked by #1203, and anyway is not readable by non-Java systems (e.g Postgres, Mongo, Redis).

@OrangeDog OrangeDog added the type: enhancement A general enhancement label Apr 14, 2025
@OrangeDog
Copy link
Author

I see that OAuth2AuthorizationServerJackson2Module is already provided, but it does not support OAuth2Authorization, which is the main thing to be persisted, and it's not found by the Core lookup.

It also duplicates some of what Spring Security Core provides (e,g. UnmodifiableMapMixin), which should probably be removed in favour of it.

@OrangeDog
Copy link
Author

OrangeDog commented Apr 15, 2025

These appear to be mostly sufficient (I'm not using OIDC, device codes, etc.). The allowlist still needs configuration.
The main difficulty is the private tokens map with Class keys, necessitating a whole Converter.
The Maven distribution omits -parameters from compilation, so mostly it's adding missing names to constructors.

class OAuth2AuthorizationConverter extends StdConverter<Map<String, Object>, OAuth2Authorization> {
    @Override
    public OAuth2Authorization convert(Map<String, Object> map) {
        RegisteredClient client = RegisteredClient.withId((String) map.get("registeredClientId"))
                .clientId("ignored")
                .authorizationGrantType(new AuthorizationGrantType("ignored"))
                .build();
        OAuth2Authorization.Builder builder = OAuth2Authorization
                .withRegisteredClient(client)
                .id((String) map.get("id"))
                .principalName((String) map.get("principalName"))
                .authorizationGrantType((AuthorizationGrantType) map.get("authorizationGrantType"))
                .authorizedScopes((Set<String>) map.get("authorizedScopes"))
                .attributes(attr -> attr.putAll((Map<String, Object>) map.get("attributes")));
        ((Map<String, OAuth2Authorization.Token<?>>) map.get("tokens")).forEach((key, value) ->
                builder.token(value.getToken(), meta -> meta.putAll(value.getMetadata())));
        return builder.build();
    }
}

@JsonAutoDetect(
        fieldVisibility = JsonAutoDetect.Visibility.ANY,
        getterVisibility = JsonAutoDetect.Visibility.NONE,
        isGetterVisibility = JsonAutoDetect.Visibility.NONE
)
@JsonDeserialize(converter = OAuth2AuthorizationConverter.class)
abstract class OAuth2AuthorizationMixin { }

@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY)
abstract class AuthorizationGrantTypeMixin { }

@JsonIncludeProperties({"token", "metadata"})
abstract class TokenMixin<T extends OAuth2Token> {
    @ConstructorProperties({"token", "metadata"})
    TokenMixin(T token, Map<String, Object> metadata) {}
}

abstract class AccessTokenMixin {
    @ConstructorProperties({"tokenType", "tokenValue", "issuedAt", "expiresAt", "scopes"})
    public AccessTokenMixin(
            TokenType tokenType, String tokenValue, Instant issuedAt, Instant expiresAt, Set<String> scopes
    ) {}
}

abstract class RefreshTokenMixin {
    @ConstructorProperties({"tokenValue", "issuedAt", "expiresAt"})
    public RefreshTokenMixin(String tokenValue, Instant issuedAt, Instant expiresAt) {}
}

I looked at using @JsonValue for AuthorizationGrantType, but that breaks the existing OAuth2AuthorizationRequestDeserializer.

@jgrandja
Copy link
Collaborator

@OrangeDog Please see comment. Closing as duplicate.

@jgrandja jgrandja self-assigned this Apr 30, 2025
@jgrandja jgrandja added status: duplicate A duplicate of another issue and removed type: enhancement A general enhancement labels Apr 30, 2025
@OrangeDog
Copy link
Author

You mean this bit?

Furthermore, I am very reluctant on providing Json utility classes and/or mappers in the core module as this is not something we want to maintain.

First, you already do, it's just not complete.

Secondly, all of us don't want to individually maintain it either. But if you leave an issue open and actually let someone else contribute this stuff (like I have above) then we will all benefit.

Thirdly, having this code and tests for it in the library will prevent you making further design decisions that make what all your users are trying to do harder than it should be.

@jgrandja
Copy link
Collaborator

@OrangeDog Hopefully the reasoning in gh-444 makes it more clear.

@OrangeDog
Copy link
Author

I'm not asking for any of that, just for OAuth2AuthorizationServerJackson2Module to be completed.

@jgrandja
Copy link
Collaborator

jgrandja commented May 1, 2025

just for OAuth2AuthorizationServerJackson2Module to be completed

I'm not following you? How is it incomplete?

If an OAuth2AuthorizationMixin existed in OAuth2AuthorizationServerJackson2Module, where would it be used in the framework?

FYI, all the mixins in OAuth2AuthorizationServerJackson2Module are being used by the framework (e.g. JdbcOAuth2AuthorizationService). We would not add a new mixin if it's not used by the framework. So I don't understand what you mean that it's incomplete?

@OrangeDog
Copy link
Author

All the other Jackson2Modules in Spring provide mappings for others to use, because they've thought ahead about what people would need to serialise via JSON.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

2 participants