Fix OAuth2 Client with Ditributed Session

[wangzw]

I got the following error message and after the whole day debug, I found that oauth2 client failed to save AuthorizationRequest to session in some case.

Resolved [OAuth2AuthorizationException: [authorization_request_not_found] ]

To trigger the issue, you have to:

1. With reactor server
2. Configure redis session. (or hazelcast session)
3. Some errors happened before and one or more legacy AuthorizationRequest leaved in the session

Result:
Oauth2 client will continue failing with error authorization_request_not_found

Root cause:

in ReactiveRedisOperationsSessionRepository.java

	@Override
	public Mono<Void> save(RedisSession session) {
		Mono<Void> result = session.saveChangeSessionId().and(session.saveDelta())
				.and((s) -> {
					session.isNew = false;
					s.onComplete();
				});
		if (session.isNew) {
			return result;
		}
		else {
			String sessionKey = getSessionKey(
					session.hasChangedSessionId() ? session.originalSessionId
							: session.getId());
			return this.sessionRedisOperations.hasKey(sessionKey)
					.flatMap((exists) -> exists ? result
							: Mono.error(new IllegalStateException(
									"Session was invalidated")));
		}
	}

Only the delta data session.saveDelta() will be update in redis session.

And the delta is captured by WebSession::getAttributes::setAttribute

	@Override
	public void setAttribute(String attributeName, Object attributeValue) {
		this.cached.setAttribute(attributeName, attributeValue);
		putAndFlush(getAttributeKey(attributeName), attributeValue);
	}

	private void putAndFlush(String a, Object v) {
		this.delta.put(a, v);
		flushImmediateIfNecessary();
	}

But In WebSessionOAuth2ServerAuthorizationRequestRepository.java

It get state to AuthorizationRequest map from session's attribute and update its value, And does not put the map back to session's attribute again by calling setAttribute. So redis session will not capture such change and fail to update the modification.

	@Override
	public Mono<Void> saveAuthorizationRequest(
			OAuth2AuthorizationRequest authorizationRequest, ServerWebExchange exchange) {
		Assert.notNull(authorizationRequest, "authorizationRequest cannot be null");
		return getStateToAuthorizationRequest(exchange, true)
				.doOnNext(stateToAuthorizationRequest -> stateToAuthorizationRequest.put(authorizationRequest.getState(), authorizationRequest))
				.then();
	}

	private Mono<Map<String, OAuth2AuthorizationRequest>> getStateToAuthorizationRequest(ServerWebExchange exchange, boolean create) {
		Assert.notNull(exchange, "exchange cannot be null");

		return getSessionAttributes(exchange)
			.doOnNext(sessionAttrs -> {
				if (create) {
					sessionAttrs.putIfAbsent(this.sessionAttributeName, new HashMap<String, OAuth2AuthorizationRequest>());
				}
			})
			.flatMap(sessionAttrs -> Mono.justOrEmpty(this.sessionAttrsMapStateToAuthorizationRequest(sessionAttrs)));
	}

	private Map<String, OAuth2AuthorizationRequest> sessionAttrsMapStateToAuthorizationRequest(Map<String, Object> sessionAttrs) {
		return (Map<String, OAuth2AuthorizationRequest>) sessionAttrs.get(this.sessionAttributeName);
	}

And then authorization_request_not_found will be raised since AuthorizationRequest is not in session.

[rwinch]

@wangzw I'm having trouble understanding what the exact issue is. Can you provide a complete sample?

[wangzw]

Distributed session implementation such as redis and hazelcast, only update the modified part of session data to session storage.

And the session modification is captured by calling WebSession::setAttribute. Only modify session data's value without calling setAttribute, session data will not be updated anyway.

For example:

The following code will NOT add ("key", "value") to session keyA_DICT_TYPE_SESSION_KEY's value, if use redis or hazelcast based distributed session.

WebSession session = ...
Map<String, String> data = (Map<String, String>)session.getAttribute("A_DICT_TYPE_SESSION_KEY");
data.put("key", "value");

You have to add setAttribute to the end like this.

WebSession session = ...
Map<String, String> data = (Map<String, String>)session.getAttribute("A_DICT_TYPE_SESSION_KEY");
data.put("key", "value");

// The following line looks redundant but is important for redis or hazelcast based distributed session.
// It is used to tell redis or hazelcast that session data with key "A_DICT_TYPE_SESSION_KEY" has been updated.
session.setAttribute("A_DICT_TYPE_SESSION_KEY", data);

setAttribute is overwrite by redis or hazelcast based distributed session to capture session data's modification.

For the above reason, we finally got "authorization_request_not_found" in some case since the code failed to update session's data.

[rwinch]

Thanks I re-read the issue and understand the problem. Would you be interested in putting a PR together?

[wangzw]

#6109 is filed for this issue.