Skip to content
This repository was archived by the owner on Mar 11, 2022. It is now read-only.

Session renewal improvements#491

Merged
ricellis merged 6 commits intomasterfrom
sync-session-renewal
Sep 27, 2019
Merged

Session renewal improvements#491
ricellis merged 6 commits intomasterfrom
sync-session-renewal

Conversation

@ricellis
Copy link
Copy Markdown
Member

Checklist

  • Tick to sign-off your agreement to the Developer Certificate of Origin (DCO) 1.1
  • Added tests for code changes or test/build only changes
  • Updated the change log file (CHANGES.md|CHANGELOG.md) or test/build only changes
  • Completed the PR template below:

Description

Improvements to session renewal logic.

1. Steps to reproduce and the simplest code sample possible to demonstrate the issue

Allow an IAM session to expire while running multiple client threads.

2. What you expected to happen

Session renewal to succeed.

3. What actually happened

Session renewal fails because the IAM service is rate limited and the multiple renewal attempts cause 429 responses that disable subsequent IAM auth.

Approach

The existing interceptor model was based on multiple auth renewal requests not causing a problem. Clearly that is not the case for IAM. Additionally during session renewal failures errors were logged and the auth disabled to allow requests to proceed through to the service and get a 401 response. This is the same as what would happen when the response proceeded with invalid creds, but it was confusing for support operatives to see requests without credentials.

To resolve these problems the exceptions will be thrown directly from the interceptors when there are problems re-authenticating, rather than delegating the unauth response to the server and locking will be introduced around the session renewal attempts to prevent multiple simultaneous attempts to renew the same session.

  • Refactored session renewal interceptors

    • Move more functionality in the CookieBaseInterceptor.
    • Throw more explicit exceptions for auth problems.
    • Do not disable auth methods after exceptions.
    • Prevent multiple simultaneous session renewal attempts.
  • Upgraded okhttp dependency to 3.12.5

  • Updated CHANGES

Schema & API Changes

  • No change - the exception received by callers in auth failure cases remains a CouchDbException.

Security and Privacy

  • Changes to auth process, but no change to the content of the auth requests.

Testing

  • Added new tests:
    • for multi-threaded session renewal overlap asserting that only a single session renewal attempt is made
    • IAM token 429 retry success case
    • IAM token 429 retry failure case
  • Modified existing tests:
    • Removed obsolete auth disable tests because they are no longer valid
    • Updated existing tests to assert exceptions
    • Removed IamSystemPropertyMock in favour of set/clear before/after tests because it was causing local JVM problems.
  • Changed travis from oraclejdk8 to openjdk8 because the default image has changed

Monitoring and Logging

Removed logging where exceptions are thrown instead.

Move more functionality in the CookieBaseInterceptor.
Throw more explicit exceptions for auth problems.
Do not disable auth methods after exceptions.
Prevent multiple simultaneous session renewal attempts.
Added a test for multiple threads expriencing session expiry
and trying to renew, asserting that only one renewal attempt
is made.

Added IAM token 429 retry tests for success and failure cases.
@smithsz smithsz self-assigned this Sep 27, 2019
@ricellis ricellis merged commit db6b3fe into master Sep 27, 2019
@ricellis ricellis deleted the sync-session-renewal branch September 27, 2019 15:22
@ricellis ricellis added this to the 2.next milestone Sep 30, 2019
@ricellis ricellis mentioned this pull request Sep 30, 2019
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants