fix(auth): improve OAuth token lifecycle and storage reliability

docs/oauth-client-implementation.md

@@ -20,7 +20,8 @@ seconds to milliseconds, adds the current Unix time, and returns an integer.
 margin, also converting its seconds to milliseconds. Equality counts as expired.
 
 `token_response_to_stored()` is the conversion boundary for provider responses.
-Only an absent or null expiry receives the one-hour fallback. Zero remains zero.
+Numeric strings are normalized at this boundary before validation. Only an absent
+or null expiry receives the one-hour fallback. Zero remains zero.
 During refresh, a missing refresh token or scope inherits the previous value;
 the original resource association remains attached to the replacement record.
 
@@ -34,8 +35,9 @@ because doing so would also discard refresh credentials.
 
 `FileTokenStore` delegates blocking work to `asyncio.to_thread()`. Its lock is
 selected by canonical path, so different instances targeting the same file share
-the same process-local lock. The complete load/modify/save operation holds that
-lock, preventing a stale reader from overwriting another caller's newly saved key.
+a lock from a fixed pool of 256 process-local locks. Unrelated paths can share a
+lock; memory usage does not grow with paths. The complete load/modify/save
+operation holds that lock, preventing a stale reader from overwriting another caller's newly saved key.
 
 The read path rejects symlinks, opens without following the final symlink where
 supported, requires a regular file, and tightens POSIX permissions. Missing files
@@ -56,16 +58,21 @@ serialization, synchronization and replacement to verify this property.
 
 Encryption is imported only when selected. AES-GCM, key derivation and the hex
 `iv:tag:ciphertext` representation preserve the older implementation's format.
+The static salt is named separately and PBKDF2 runs once during encrypted store
+construction. Only derived bytes are retained; IVs remain random per write.
 `token-encryption` advertises the optional dependency, and the development extra
 includes it so encryption tests always execute. Neither store coordinates
 multiple writer processes; Windows directory privacy depends on ACLs.
 
 ## Client configuration and transport
 
-`auth/client.py` defines typed, redacted config, authorization-flow and
-registration results. Constructor mappings use the same snake_case names as the
-config model. The client creates a memory store unless the caller supplies one.
-Request timeout and response limits are validated once at construction.
+`auth/_client_types.py` defines typed, redacted config, authorization-flow and
+registration results. `auth/client.py` owns the client lifecycle and transport,
+and re-exports these types to preserve existing imports. Constructor mappings
+use the same snake_case names as the config model. The client creates a memory store unless the caller supplies one.
+Request timeout and response limits are validated once at construction. Client
+secrets can remain wrapped in `SecretValue` until authentication calls
+`unwrap_secret()`; empty or unusable wrappers produce sanitized errors.
 
 The transport has two layers. `_request()` owns validation, DNS safety, status
 handling and sanitized errors. `_send()` is an asynchronous seam that delegates
@@ -120,6 +127,12 @@ The first caller creates the task; simultaneous callers await that same task.
 A completion callback removes the task and retrieves its exception, including
 when all waiters cancelled. A failed operation can therefore be retried later.
 
+`get_access_token()` shares the refresh task machinery and checks expiry under
+the per-key lock. It returns a valid credential without HTTP, or refreshes and
+persists before returning its access-token string. Explicit refresh requests
+promote a pending automatic check to a forced refresh. Completed tasks are not
+reused, and both coordination maps are cleared on completion.
+
 Different keys have different locks and refresh tasks. Same-key acquisition,
 refresh and revocation hold a shared lock within the client instance. In
 particular, revocation waits for an already-running refresh and then revokes the
@@ -134,8 +147,11 @@ save failure, which an application must handle by reauthorization if necessary.
 
 Revocation defaults to the stored refresh token when available and otherwise the
 access token. An explicit hint selects either credential. It removes the local
-record only after upstream success. Client registration sends JSON and returns a
-redacted typed result; it does not implicitly change client configuration.
+record only after upstream success by default. Opt-in `local_fallback=True` also
+deletes after missing endpoints or remote failures, without claiming remote
+invalidation. Storage errors, invalid hints and cancellation still propagate.
+Refresh rotation across independent clients or processes remains unsupported.
+Client registration sends JSON and returns a redacted typed result; it does not implicitly change client configuration.
 
 ## Error boundaries and verification
 
@@ -148,6 +164,5 @@ tracebacks, logs, stdout and stderr. Explicit serialization remains secret-beari
 The three new test modules cover storage/expiry, OAuth lifecycle/concurrency, and
 urllib/pinned transport behavior respectively. They use fake upstream responses
 and controlled coordination events instead of live providers. Existing OAuth,
-PKCE, CIMD, auth and full-suite tests exercise integration with the SDK. See the
-separate verification report for exact platform results and the Windows socket
-limitation; local Linux runs are not a claim of hosted CI success.
+PKCE, CIMD, auth and full-suite tests exercise integration with the SDK. Current
+validation results belong in the PR description rather than a permanent report.

docs/oauth-client.md

@@ -11,7 +11,7 @@ it. It does not launch browsers, receive callbacks, or intercept API requests.
 import asyncio
 import os
 
-from nitrostack import OAuth2Client, OAuth2ClientConfig, is_token_expired
+from nitrostack import OAuth2Client, OAuth2ClientConfig
 
 
 async def main():
@@ -21,13 +21,10 @@ async def main():
         token_endpoint="https://identity.example.com/oauth/token",
     ))
     key = "identity.example.com|reporting-app|service-account|reports-api"
-    token = await client.store.get_token(key)
-    if token is None or is_token_expired(token):
-        token = await client.get_client_credentials_token(
-            key, scope="reports:read", resource="https://reports.example.com",
-        )
-    # Pass token.access_token to your HTTP client's Authorization header.
-    # Do not print the credential or its serialized representation.
+    token = await client.get_client_credentials_token(
+        key, scope="reports:read", resource="https://reports.example.com",
+    )
+    # Use token.access_token without logging it.
 
 
 asyncio.run(main())
@@ -148,26 +145,29 @@ store = FileTokenStore(
 Encryption imports are lazy. The file format remains compatible with PR #4:
 AES-GCM with a random 16-byte IV, hex `iv:tag:ciphertext`, and its existing
 PBKDF2-HMAC-SHA256 key derivation (100,000 iterations, `nitrostack-token-store`
-salt). Keep the encryption key separate from the file. Switching an existing
+salt). Derivation runs once per encrypted store instance at construction; only
+the derived key is retained. Every write uses a fresh IV. Keep the encryption key
+separate from the file. Switching an existing
 plaintext file to encryption requires an explicit migration by the application.
 
 ## Expiry and refresh
 
 `StoredToken` is immutable. `expires_at` is an integer Unix timestamp in
 **milliseconds**. `calculate_expiration(expires_in)` and `is_token_expired(token,
 skew=30)` take **seconds**. Missing/`None` `expires_in` uses one hour; zero expires
-immediately. Negative, nonnumeric, boolean and nonfinite durations are rejected.
+immediately. Provider responses also accept numeric strings such as `"3600"`
+and `"1.5"`. Negative, nonnumeric, boolean and nonfinite durations are rejected.
+The numeric-only expiry helpers do not accept strings.
+
+`get_access_token(key)` returns a string, reuses valid stored tokens, and refreshes
+within the 30-second safety margin. Missing records or refresh credentials raise
+`OAuthClientError`; the application must then acquire a token explicitly.
 
 Stores retain expired records so the refresh credential remains available:
 
 ```python
-token = await client.store.get_token(key)
-if token is not None and is_token_expired(token):
-    if token.refresh_token:
-        token = await client.refresh_token(key)
-    else:
-        # Start the appropriate acquisition flow again.
-        ...
+access_token = await client.get_access_token(key)
+# Use the returned credential without logging it.
 ```
 
 Share one client instance to coalesce concurrent refresh calls for a key into one
@@ -185,6 +185,18 @@ and deletes the local record only after upstream success. Pass
 successful revocation response is accepted. Coordination is not shared between
 client instances or processes.
 
+Pass `local_fallback=True` to delete the local record when the revocation endpoint
+is missing or the remote attempt fails. This does **not** guarantee the provider
+invalidated the credential. Invalid hints and storage errors still raise, and
+cancellation propagates. For unconditional local deletion without a request, use
+`await client.store.delete_token(key)`.
+
+File operations share a fixed pool of 256 process-local locks selected by canonical
+path. Unrelated paths may serialize when they select the same lock. This bounds
+lock memory but does not coordinate refresh rotation across client instances or
+processes. Use one shared client per credential owner; do not run concurrent
+refresh-token rotation against the same file from separate processes.
+
 ## Errors and secret handling
 
 Catch `OAuthClientError` and `TokenStoreError` for sanitized failures. OAuth errors
@@ -196,3 +208,19 @@ Config, token, pending-flow and registration representations hide credentials.
 Explicit `token.to_dict()` and `registration.to_dict()` contain credentials for
 interoperability/persistence: **never log these mappings**. Redacted repr does not
 make arbitrary attribute access, application logs or memory dumps safe.
+
+### Wrapped client secrets
+
+```python
+from nitrostack import OAuth2Client, OAuth2ClientConfig, SecretValue
+
+client = OAuth2Client(OAuth2ClientConfig(
+    client_id="my-client",
+    client_secret=SecretValue.from_env("OAUTH_CLIENT_SECRET"),
+    token_endpoint="https://issuer.example/token",
+))
+```
+
+Mappings also accept `SecretValue`. The wrapper stays in configuration and is
+unwrapped only while constructing authentication. Empty or serialized/redacted
+wrappers fail safely before a request. Plain-string secrets remain supported.

nitrostack/auth/_client_types.py

@@ -0,0 +1,98 @@
+"""Secret-safe OAuth configuration, results, errors, and transport records."""
+
+from __future__ import annotations
+
+from dataclasses import dataclass, field, fields
+from types import MappingProxyType
+from typing import Any, Mapping
+
+from nitrostack.auth.secret import SecretValue
+
+
+_OAUTH_ERRORS = frozenset({
+    "invalid_request", "invalid_client", "invalid_grant", "unauthorized_client",
+    "unsupported_grant_type", "invalid_scope", "access_denied", "server_error",
+    "temporarily_unavailable", "invalid_token", "unsupported_token_type",
+    "invalid_redirect_uri", "invalid_client_metadata",
+})
+
+
+class OAuthClientError(ValueError):
+    """Only operation, status, and recognized OAuth error codes are exposed."""
+
+    def __init__(self, operation: str, *, status: int | None = None, error_code=None):
+        self.operation = operation
+        self.status = status
+        self.error_code = error_code if isinstance(error_code, str) and error_code in _OAUTH_ERRORS else None
+        suffix = f" (HTTP {status})" if status is not None else ""
+        if self.error_code:
+            suffix += f": {self.error_code}"
+        super().__init__(f"OAuth {operation} failed{suffix}")
+
+
+@dataclass(frozen=True)
+class OAuth2ClientConfig:
+    client_id: str | None = field(default=None, repr=False)
+    client_secret: str | SecretValue | None = field(default=None, repr=False)
+    issuer: str | None = field(default=None, repr=False)
+    redirect_uri: str | None = field(default=None, repr=False)
+    authorization_endpoint: str | None = field(default=None, repr=False)
+    token_endpoint: str | None = field(default=None, repr=False)
+    revocation_endpoint: str | None = field(default=None, repr=False)
+    registration_endpoint: str | None = field(default=None, repr=False)
+    protected_resource_metadata_url: str | None = field(default=None, repr=False)
+    authorization_server_metadata_url: str | None = field(default=None, repr=False)
+
+    def __post_init__(self):
+        if any(value is not None and (not isinstance(value, str) or not value)
+               for value in (getattr(self, item.name) for item in fields(self) if item.name != "client_secret")):
+            raise OAuthClientError("configuration")
+        if self.client_secret is not None and not isinstance(self.client_secret, SecretValue):
+            if not isinstance(self.client_secret, str) or not self.client_secret:
+                raise OAuthClientError("configuration")
+
+
+@dataclass(frozen=True)
+class AuthorizationFlow:
+    authorization_url: str = field(repr=False)
+    state: str = field(repr=False)
+    code_verifier: str = field(repr=False)
+    code_challenge: str = field(repr=False)
+    code_challenge_method: str = "S256"
+
+
+@dataclass(frozen=True)
+class ClientRegistration:
+    client_id: str = field(repr=False)
+    client_secret: str | None = field(default=None, repr=False)
+    metadata: Mapping[str, Any] = field(default_factory=dict, repr=False)
+
+    def __post_init__(self):
+        if (not isinstance(self.client_id, str) or not self.client_id
+                or (self.client_secret is not None and not isinstance(self.client_secret, str))):
+            raise OAuthClientError("registration response")
+        object.__setattr__(self, "metadata", MappingProxyType(dict(self.metadata)))
+
+    def to_dict(self) -> dict[str, Any]:
+        """Explicit credential-bearing output; never log this mapping."""
+        result = {**self.metadata, "client_id": self.client_id}
+        if self.client_secret is not None:
+            result["client_secret"] = self.client_secret
+        return result
+
+
+@dataclass(frozen=True)
+class _Request:
+    method: str
+    url: str = field(repr=False)
+    headers: Mapping[str, str] = field(repr=False)
+    body: bytes | None = field(repr=False)
+    timeout: float
+    pinned_ip: str | None = field(default=None, repr=False)
+
+
+@dataclass(frozen=True)
+class _Response:
+    status: int
+    headers: Mapping[str, str] = field(repr=False)
+    body: bytes = field(repr=False)