Add OAuth 2.0 access-token authentication for on-premises NoSQL - #216
Add OAuth 2.0 access-token authentication for on-premises NoSQL#216rajdeep714 wants to merge 16 commits into
Conversation
Schedule reauthentication using the earlier of the OAuth access-token expiry and the NoSQL login-token expiry returned by the proxy. Preserve the current login session when proactive refresh fails and use the request timeout for request-driven login. Add regression coverage for shorter KV sessions, failed refresh callbacks, and OAuth login timeouts.
oracle-contributor-agreement
Bot
added
the
OCA Verified
Parse the structured authenticatedIdentity returned by /oauthlogin and bind each provider instance to the canonical issuer, subject type, and stable subject ID established by KV. Reject missing, malformed, or changed identities and perform best-effort logout of a rejected candidate session. Keep the SDK provider-neutral and independent of KV implementation classes; it does not parse JWT claims. Cover same and changed issuer, subject type, and subject ID, missing identity, refresh and relogin, and logout behavior.
OAuth login creates an ordinary NoSQL login session, so provider cleanup now uses the existing /logout endpoint. This preserves mixed-version compatibility and makes clear that closing the provider cleans up the KV session but does not revoke the original identity-provider access token.
| sendRequest(logoutAuth, LOGOUT_SERVICE, timeoutMs); | ||
| if (response.getStatusCode() != HttpResponseStatus.OK.code() && | ||
| logger != null) { | ||
| logger.info("Failed to logout OAuth session, response: " + |
There was a problem hiding this comment.
Please revisit the logging in this class, either redact or remove tokens, remote bodies or arbitrary exception messages. There were several findings before.
There was a problem hiding this comment.
Updated. OAuth login/logout now use the HTTP helper without passing the configured logger to the lower HTTP layer. Provider logs retain only the operation, HTTP status, and exception class; they no longer include access tokens, response bodies, or arbitrary exception messages.
| */ | ||
| protected abstract AccessTokenInfo getAccessTokenInfo(); | ||
|
|
||
| private synchronized void performLogin(boolean force, Request request) { |
There was a problem hiding this comment.
performLogin() holds the provider monitor while it invokes application code.
Because the monitor is reentrant, that callback can call close(), set
isClosed, and return normally. The method then constructs and sends
oauthlogin; its next closed-state check is after the response arrives. In
that sequence, a closed provider can still create a server session and discard
it without a corresponding logout.
Recheck isClosed after validating the callback result and immediately before
constructing or sending the OAuth request. This narrow guard closes the
demonstrated path without changing normal callback ownership.
There was a problem hiding this comment.
Updated. performLogin() now rechecks closed/generation state after getAccessToken() returns and immediately before sending /oauthlogin. If the callback closes the provider, login stops without creating a NoSQL session.
| } | ||
| authString.set(BEARER_PREFIX + loginResult.getToken()); | ||
| tokenInfo = newTokenInfo; | ||
| accessTokenExpireAt = accessTokenAcquireTime + |
There was a problem hiding this comment.
The provider combines two independently supplied lifetimes: callback
expiresInSeconds and proxy expireAt. Long.MAX_VALUE seconds converts to a
saturated millisecond value and overflows when added to the current epoch; the
result can make scheduleRefresh() use its one-second floor. A positive proxy
expiry that is already past has the same practical effect. These values need
not be malicious to occur after a unit mismatch or bad clock conversion.
Use checked arithmetic (or explicitly reject a lifetime that cannot form a
future millisecond deadline), and require an expiry used for renewal to be
future. Preserve a documented non-renewing behavior only for an intentionally
non-expiring/zero lifetime; do not silently treat malformed values as a
one-second refresh schedule.
There was a problem hiding this comment.
Updated with an API simplification. The callback now returns only a usable access token through getAccessToken(); AccessTokenInfo, callback-provided expiresIn, and client-side lifetime arithmetic were removed. This eliminates overflow and stale-lifetime concerns.
| * responses are surfaced as authentication failures | ||
| * instead of eventually timing out the request. | ||
| */ | ||
| if (retriedException(kvRequest, |
There was a problem hiding this comment.
The OAuth-specific AuthenticationException and the existing
InvalidAuthorizationException path each ask retry statistics whether their
own concrete class has already occurred. A request that receives one class and
then the other can therefore obtain two authentication retries, two cache
invalidations, and three data attempts. The issue applies only where both
classes represent the same expired/invalid OAuth credential, not to unrelated
non-OAuth authorization errors.
Use one OAuth-authentication retry marker or allowance for both paths, without
changing retry policy for other providers.
There was a problem hiding this comment.
Done. OAuth AuthenticationException and InvalidAuthorizationException now share one retry allowance.
| rae.getMessage()); | ||
| throw rae; | ||
| } | ||
| authProvider.flushCache(); |
There was a problem hiding this comment.
Each request obtains an authorization string before sending the data request,
but both OAuth authentication-error paths unconditionally call
authProvider.flushCache(). The provider then clears the shared authorization
string:
authString.set(null);
If request T0 fails using token T0 after another concurrent request has already
installed token T1, the delayed T0 failure clears healthy T1. The next
retry performs an unnecessary OAuth callback and login, causing avoidable
authentication load and transient availability failures during token rotation
or repeated stale responses.
Add an OAuth-specific compare-and-set invalidation operation that clears the
cache only when the current authorization value still equals the value
used by the failed request. If the value has changed, preserve the newer token
and let the retry use it. Apply this to both the AuthenticationException
path at Client.java:868-869 and the InvalidAuthorizationException path at
Client.java:901-904.
There was a problem hiding this comment.
Updated. Client now passes the exact authorization string used by the failed request to invalidateAuthorizationString(). The provider clears the cache with compare-and-set, so a delayed failure for an older token cannot remove a newer cached token.
| endpoint = null; | ||
| loginPort = 0; | ||
| logger = null; | ||
| scheduler = Executors.newSingleThreadScheduledExecutor(r -> { |
There was a problem hiding this comment.
The scheduler returned by Executors.newSingleThreadScheduledExecutor()
delegates to a ScheduledThreadPoolExecutor, which retains cancelled delayed
tasks until their delay elapses unless remove-on-cancel is enabled. A successful
login schedules one refresh; flushCache() followed by lazy re-login cancels
it and schedules another. Repeating that sequence before a long expiry retains
one cancelled task per re-login. This needs repeated invalidation and long
delays, so it is an operational memory-growth risk rather than an immediate
per-request failure.
Use a private one-thread ScheduledThreadPoolExecutor with the existing daemon
thread factory and setRemoveOnCancelPolicy(true). That addresses queued
cancelled tasks only; keep the separate generation guard for a task that is
already running.
There was a problem hiding this comment.
Updated. The provider now uses a one-thread ScheduledThreadPoolExecutor with setRemoveOnCancelPolicy(true). Replacing, invalidating, disabling renewal, or closing cancels the queued task immediately; the generation guard handles a task that has already started.
Prevent OAuth login and refresh from creating or retaining sessions after close, cache invalidation, or auto-renewal changes. Use checked token-expiry arithmetic, reject expired proxy login tokens, and clean up rejected sessions. Use conditional OAuth cache invalidation and one shared authentication retry allowance so stale failures cannot clear a newer token or obtain extra retries. Send OAuth login and logout through a single-attempt HTTP path and keep tokens, response bodies, and arbitrary exception messages out of OAuth logs. Remove cancelled refresh tasks promptly, exclude mock HTTPS OAuth tests from server-backed profiles, and add focused coverage for callback close, expiry validation, refresh cancellation, retry races, log redaction, and single-attempt failures.
Use the KV-authenticated NoSQL session expiration as the single source of truth for SDK OAuth re-login scheduling. Replace AccessTokenInfo with a string-only getAccessToken callback so applications manage token acquisition without supplying duplicate or potentially stale lifetime metadata. Require HTTPS handle configuration even when an OAuth endpoint is supplied explicitly, complete missing SSL settings from NoSQLHandleConfig, and validate oauthlogin response fields strictly. Reject non-future login-token deadlines and best-effort logout sessions returned in malformed or identity-mismatched responses. Normalize OAuth retry accounting to base authentication exception classes so exception subclasses cannot bypass the shared one-retry allowance. Preserve conditional cache invalidation and refresh-generation cancellation behavior. Update unit coverage for response validation, TLS preparation, server-driven refresh timing, callback and refresh races, retry subclasses, cleanup, and sensitive logging. Update the on-prem OAuth example and README for the string-only callback API. Validation: focused OAuth and retry unit tests, examples reactor packaging, and driver Javadocs.
|
@yfei-a I made an API simplification while resolving your comments. The callback now returns only the access token through The callback lifetime was not authoritative and could be stale or incorrectly calculated. KV already validates the OAuth token expiration, limits the NoSQL session to the earlier of the validated token expiry and KV session timeout, and returns that deadline as This keeps the callback provider-neutral, supports opaque tokens, and removes the original-versus-remaining lifetime and overflow ambiguity. Could you please review the latest changes? |
| } | ||
| final HttpHeaders headers = new DefaultHttpHeaders(); | ||
| headers.set(AUTHORIZATION, authHeader); | ||
| client = HttpClient.createMinimalClient |
There was a problem hiding this comment.
When NoSQLHandleConfig specifies an HTTP proxy, the main client calls configureProxy, but this minimal client does not. Consequently /oauthlogin, refresh, and logout requests connect directly and fail in environments where the configured proxy is the only route to the service, even though normal database requests are routed correctly.
There was a problem hiding this comment.
Updated OAuthAccessTokenProvider to set proxy configuration in the HttpClient before sending a request. For a side note, StoreAccessTokenProvider also uses a minimal client without configuring proxy.
| if (loginAborted(force, expectedGeneration)) { | ||
| return; | ||
| } | ||
| final int timeoutMs = |
There was a problem hiding this comment.
When re-login follows a database attempt that consumed most of the request budget, request.getTimeoutInternal() still contains the original timeout rather than the remaining time computed by Client. The OAuth exchange can therefore block for another full timeout, causing an operation to substantially exceed its configured request timeout.
There was a problem hiding this comment.
Fixed. OAuth login now subtracts the elapsed time since Request.getStartNanos() from the configured request timeout. An exhausted budget throws RequestTimeoutException. Side note that this issues also exists in StoreAccessTokenProvider.
| if (this.autoRenew == autoRenew) { | ||
| return this; | ||
| } | ||
| this.autoRenew = autoRenew; |
There was a problem hiding this comment.
When callers concurrently toggle renewal from true to false and back to true, the enabling caller can schedule a refresh before the disabling caller reaches invalidateRefreshTask; the latter then cancels that new task, leaving autoRenew == true with no refresh scheduled. The current token will consequently not be renewed until a server-side authentication failure forces re-login.
There was a problem hiding this comment.
Fixed. Automatic-renew state and refresh-task changes are now serialized under a dedicated refresh-state lock. This prevents a disabling caller from cancelling a newer task scheduled by an enabling caller, without making setAutoRenew(false) wait for a running getAccessToken callback.
- OAuth login, refresh, cleanup, and logout now inherit the handle's HTTP proxy configuration. - Foreground OAuth login uses the operation's remaining timeout and fails immediately when exhausted. - Auto-renew state and task transitions use a dedicated lock, avoiding the race without blocking behind getAccessToken().
|
@connelly38 @jinzha @yfei-a @xiaoy-nosql a gentle reminder to review this PR when you get a chance. Thanks for your time and help! |
yfei-a
left a comment
yfei-a
left a comment
There was a problem hiding this comment.
@rajdeep714, thanks, overall looks good to me. Few minor comments.
- Handle HTTP 5xx login responses as retryable SystemExceptions. - Preserve NoSQLException subtypes from OAuth login failures. - Avoid authorization cache races by returning a stable authorization snapshot from performLogin(). - Use the cached authorization value from a single atomic read. - Modernize AtomicReference initialization with the diamond operator. - Use equalsIgnoreCase() for HTTPS protocol validation. - Remove the unnecessary checked Exception declaration from sendRequest().
3 participants
access token for a NoSQL login token through the HTTP proxy.
OCI IAM is the supported identity provider for the initial integration, while
the SDK API remains provider-neutral.
login responses that omit the identity or switch issuer, subject type, or
subject ID without logout.
by the server. KV bounds this expiration by the validated OAuth token expiry
and the configured store session timeout.
authorization failure, using one shared retry allowance.
a delayed failure cannot discard a newer token.
avoid repeated HTTP attempts during login/logout exchanges, and exclude
credential contents from logs.
OAuth sessions.
Common and the -useOAuth option.
Reviewed by: Ashutosh, Jin, John, Xiao, Yang
API impact:
longer provide separate token-lifetime metadata.
Tests:
-Dtest=OAuthAccessTokenProviderTest,AuthRetryTest test
tests.
Dependencies:
Files (Added / Modified / Deleted):
M README.md
token supplied through an environment variable.
that the SDK schedules re-login from the server-returned expiration.
M driver/pom.xml
it from the three server-backed profiles because it owns a local HTTPS test
endpoint.
M driver/src/main/java/oracle/nosql/driver/http/Client.java
and request-size enforcement.
retry allowance and normalizes exception subclasses to prevent extra retries.
invalidation, preserving a newer token during request/refresh races.
M driver/src/main/java/oracle/nosql/driver/http/NoSQLHandleImpl.java
endpoint, SSL context, and handshake timeout.
configured before handle creation.
A driver/src/main/java/oracle/nosql/driver/kv/OAuthAccessTokenProvider.java
and schedules re-login from the server-returned session expiration.
and performs best-effort cleanup when a usable rejected token is available.
invalidation, refresh generations, and immediate scheduled-task cancellation.
exception messages.
M driver/src/main/java/oracle/nosql/driver/util/HttpRequestUtil.java
failure without retrying until timeout, while preserving existing behavior
for other HTTP utility callers.
M driver/src/test/java/oracle/nosql/driver/iam/AuthRetryTest.java
InvalidAuthorizationException, including alternating errors and subclasses.
A driver/src/test/java/oracle/nosql/driver/kv/OAuthAccessTokenProviderTest.java
renewal, refresh failure, and cache invalidation.
malformed/expired responses, callback and refresh races, HTTPS preparation,
scheduled-task cancellation, sensitive logging, and logout cleanup.
M examples/src/main/java/Common.java
and RateLimitingExample.
combinations, and reads the example access token from
NOSQL_OAUTH_ACCESS_TOKEN.
the example supplies one static token.