spring-attic / spring-security-oauth

Support for adding OAuth1(a) and OAuth2 features (consumer and provider) for Spring web applications.
http://github.com/spring-projects/spring-security-oauth
Apache License 2.0
4.69k stars 4.04k forks source link

DuplicateKeyException under load #1033

Open bilak opened 7 years ago

bilak commented 7 years ago

Hello, when creating access token with same user/client there's possible issue which will result into DuplicateKeyException. I've tested this on Oracle, Postgre and H2 databses. I've been trying to solve this with details in #502 . After this implementation Oracle and H2 works ok but there is strange issue with postgre because it has ?different semantics? of transaction.

there is a proxy self-invocation (the transactional method calls another method on the same object) Postgres, quite reasonably, doesn't like that. It's telling you that the transaction is dead, so you need to start again.

So @dsyer suggested to implement workaround with @Retryable. Can someone please look into this and probably find the way how to correctly implement this in spring-security-oauth project for all databses? If more details is needed, just let me know.

Thanks

kflorian commented 7 years ago

I can confirm the issue exists, using PostgreSQL DB 9.6 and spring-security-oauth version 2.0.12.RELEASE.

sanuj5 commented 5 years ago

After spring boot 2.0.5.RELEASE upgrade, I get issues with oracle also. It was working with 1.5.15.RELEASE without any issue.

I have 3 instances of authorization server behind zuul proxy. I did load test with 3 simultaneous scripts, each scripts having 500 curl requests to gateway. Not a single failure for 1.5.15.RELEASE. But too many failures with 2.0.5.RELEASE and token changes frequently. Token expiry is set to 1 day for both cases, so it's not during token expiring/refreshing time.

bmaehr commented 3 years ago

I think the problem is https://github.com/spring-projects/spring-security-oauth/blob/master/spring-security-oauth2/src/main/java/org/springframework/security/oauth2/provider/token/store/JdbcTokenStore.java#L148 and 149 using token_id instead of authentication_id:

if (readAccessToken(token.getValue())!=null) {
    removeAccessToken(token.getValue());
}

This should be a workaround:

    @Bean
    public TokenStore tokenStore(final DataSource dataSource) {
        final JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource);
        final AuthenticationKeyGenerator authenticationKeyGenerator = new DefaultAuthenticationKeyGenerator();
        return new JdbcTokenStore(dataSource) {

            @Override
            public void storeAccessToken(final OAuth2AccessToken token, final OAuth2Authentication authentication) {
                final String key = authenticationKeyGenerator.extractKey(authentication);
                jdbcTemplate.update("delete from oauth_access_token where authentication_id = ?", key);
                super.storeAccessToken(token, authentication);
            }

        };
    }
bmaehr commented 3 years ago

Because there was in #502 the question about a use case: Browser renders a list and does an additional request for each item to the backend running on multiple nodes. The backend nodes use the same database but doesn't answer these requests directly and forwards them over an oauth2 client authentication request to a second backend service.

alexbaxter commented 3 years ago

I've just hit this problem in a project (I think) - does anyone know if is it likely to be worked on in the Spring library? If not I will add the patched token service in my code.