Skip to content

Let an expired session renew itself at the provider #1178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jviotti merged 5 commits into from
Jul 30, 2026
Merged

Let an expired session renew itself at the provider #1178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6926f21
Let an expired session renew itself at the provider
jviotti Jul 30, 2026
286eb08
Fix
jviotti Jul 30, 2026
7117635
More
jviotti Jul 30, 2026
c01b40c
Fix
jviotti Jul 30, 2026
8c6fdc9
Fix
jviotti Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ follows is signed with a secret of the instance's own, unrelated to the provider
| `/clientId` | String | :red_circle: **Yes** | N/A | The client identifier registered with the provider for this instance |
| `/clientSecret` | Object | :red_circle: **Yes** | N/A | The client secret shared with the provider, read from an environment variable so that it never lives in the configuration file |
| `/clientSecret/environmentVariable` | String | :red_circle: **Yes** | N/A | The name of the environment variable that holds the client secret |
| `/sessionSecrets` | Array | :red_circle: **Yes** | N/A | The secrets used to sign the session cookies this instance mints, newest first. These are the instance's own secrets, unrelated to the provider. A cookie is signed under the first and accepted under any, so adding a new secret first and dropping the old one once the sessions signed under it have expired rotates without signing anybody out |
| `/sessionSecrets` | Array | :red_circle: **Yes** | N/A | The secrets used to sign the session cookies this instance mints, newest first. These are the instance's own secrets, unrelated to the provider. A cookie is signed under the first and accepted under any, so adding a new secret first and dropping the old one once the sessions signed under it have expired rotates without signing anybody out. Unlike a `jwt` key set, these are read from the environment once at startup, so a change to them takes effect on restart |
| `/sessionSecrets/*` | Object | :red_circle: **Yes** | N/A | A single session signing secret |
| `/sessionSecrets/*/environmentVariable` | String | :red_circle: **Yes** | N/A | The name of the environment variable that holds the session signing secret. Generate it at random, with at least 32 characters, as with `openssl rand -base64 32`. Everything a session cookie carries but its signature travels in the open, so a secret that can be guessed is one that anybody holding a single cookie can find, after which they can mint sessions of their own |

Expand Down
43 changes: 43 additions & 0 deletions enterprise/authentication/authentication.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,43 @@ struct Authentication::Impl {
decoded.default_path};
}

[[nodiscard]] auto interactive(const std::string_view path,
const std::string_view name) const
-> std::optional<Authentication::InteractivePolicy> {
const auto mask{this->match(path)};
if (mask == 0 || this->policy_count_ == 0 || name.empty()) {
return std::nullopt;
}

const auto *policies{
static_cast<const AuthenticationPolicyEntry *>(this->policies_)};
for (std::uint32_t index{0}; index < this->policy_count_; index += 1) {
if ((mask & (PolicySet{1} << index)) == 0) {

@cubic-dev-ai cubic-dev-ai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Silent renewal can redirect a path to /auth/login/{name} even when that endpoint resolves the same duplicate name to a different policy. Preserve the first-match name semantics here (then test whether that entry governs path), or reject duplicate OIDC names when serializing.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At enterprise/authentication/authentication.cc, line 835:

<comment>Silent renewal can redirect a path to `/auth/login/{name}` even when that endpoint resolves the same duplicate name to a different policy. Preserve the first-match name semantics here (then test whether that entry governs `path`), or reject duplicate OIDC names when serializing.</comment>

<file context>
@@ -821,6 +821,43 @@ struct Authentication::Impl {
+    const auto *policies{
+        static_cast<const AuthenticationPolicyEntry *>(this->policies_)};
+    for (std::uint32_t index{0}; index < this->policy_count_; index += 1) {
+      if ((mask & (PolicySet{1} << index)) == 0) {
+        continue;
+      }
</file context>

continue;
}

const auto &entry{policies[index]};
if (static_cast<Authentication::Type>(entry.type) !=
Authentication::Type::OIDC ||
entry.metadata_length == 0) {
continue;
}

const std::span<const std::byte> metadata{
this->view_->as<std::byte>(entry.metadata_offset),
entry.metadata_length};
OIDCPolicyMetadata decoded;
if (decode_oidc_metadata(metadata, decoded) && decoded.name == name) {
return Authentication::InteractivePolicy{.issuer = decoded.issuer,
.client_id = decoded.client_id,
.default_path =
decoded.default_path};
}
}

return std::nullopt;
}

[[nodiscard]] auto client_secret(const std::string_view policy) const
-> std::optional<sourcemeta::core::SecureString> {
OIDCPolicyMetadata decoded;
Expand Down Expand Up @@ -1195,6 +1232,12 @@ auto Authentication::interactive(const std::string_view name) const
return this->impl_->interactive(name);
}

auto Authentication::interactive(const Authentication::Path &path,
const std::string_view name) const
-> std::optional<Authentication::InteractivePolicy> {
return this->impl_->interactive(path.value(), name);
}

auto Authentication::client_secret(const std::string_view policy) const
-> std::optional<sourcemeta::core::SecureString> {
return this->impl_->client_secret(policy);
Expand Down
3 changes: 2 additions & 1 deletion enterprise/e2e/auth-closed/hurl/sso.all.hurl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ POST {{base}}/self/v1/auth/logout
HTTP 303
Cache-Control: no-store
[Asserts]
header "Set-Cookie" count == 2
header "Set-Cookie" count == 3
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
cookie "sourcemeta_one_session[Max-Age]" == 0
cookie "sourcemeta_one_transaction[Max-Age]" == 0
cookie "sourcemeta_one_renewal[Max-Age]" == 0
header "Location" startsWith "https://keycloak:8443/realms/main/protocol/openid-connect/logout?"

# With the session gone the catalog is locked to the browser again, the denial
Expand Down
4 changes: 3 additions & 1 deletion enterprise/e2e/auth-path/hurl/sso.all.hurl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,13 @@ POST {{base}}/registry/self/v1/auth/logout
HTTP 303
Cache-Control: no-store
[Asserts]
header "Set-Cookie" count == 2
header "Set-Cookie" count == 3
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
cookie "sourcemeta_one_session[Max-Age]" == 0
cookie "sourcemeta_one_session[Path]" == "/registry"
cookie "sourcemeta_one_transaction[Max-Age]" == 0
cookie "sourcemeta_one_transaction[Path]" == "/registry"
cookie "sourcemeta_one_renewal[Max-Age]" == 0
cookie "sourcemeta_one_renewal[Path]" == "/registry"
header "Location" startsWith "https://keycloak:8443/realms/main/protocol/openid-connect/logout?"
# The instance URL already carries the base path, so it is named once
header "Location" contains "post_logout_redirect_uri=http%3A%2F%2Flocalhost%3A8000%2Fregistry"
Expand Down
3 changes: 2 additions & 1 deletion enterprise/e2e/auth-sso/hurl/login.all.hurl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,10 @@ POST {{base}}/self/v1/auth/logout
HTTP 303
Cache-Control: no-store
[Asserts]
header "Set-Cookie" count == 2
header "Set-Cookie" count == 3
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
cookie "sourcemeta_one_session[Max-Age]" == 0
cookie "sourcemeta_one_transaction[Max-Age]" == 0
cookie "sourcemeta_one_renewal[Max-Age]" == 0
header "Location" startsWith "https://keycloak:8443/realms/main/protocol/openid-connect/logout?"

# And the private catalog is gated again, the denial byte-identical to the first
Expand Down
11 changes: 8 additions & 3 deletions enterprise/e2e/auth-sso/hurl/logout.all.hurl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ HTTP 303
Cache-Control: no-store
Location: /
[Asserts]
header "Set-Cookie" count == 2
header "Set-Cookie" count == 3
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
cookie "sourcemeta_one_session[Max-Age]" == 0
cookie "sourcemeta_one_session[HttpOnly]" exists
cookie "sourcemeta_one_session[Path]" == "/"
cookie "sourcemeta_one_transaction[Max-Age]" == 0
cookie "sourcemeta_one_transaction[HttpOnly]" exists
cookie "sourcemeta_one_transaction[Path]" == "/"
cookie "sourcemeta_one_renewal[Max-Age]" == 0
cookie "sourcemeta_one_renewal[HttpOnly]" exists
cookie "sourcemeta_one_renewal[Path]" == "/"

# A value that is not a session this instance minted is cleared just the same,
# and discloses nothing by being treated differently
Expand All @@ -30,9 +33,10 @@ HTTP 303
Cache-Control: no-store
Location: /
[Asserts]
header "Set-Cookie" count == 2
header "Set-Cookie" count == 3
cookie "sourcemeta_one_session[Max-Age]" == 0
cookie "sourcemeta_one_transaction[Max-Age]" == 0
cookie "sourcemeta_one_renewal[Max-Age]" == 0

# A real login, so there is a genuine session to end
GET {{base}}/self/v1/auth/login/keycloak
Expand Down Expand Up @@ -81,9 +85,10 @@ Cache-Control: no-store
[Captures]
provider_logout: header "Location"
[Asserts]
header "Set-Cookie" count == 2
header "Set-Cookie" count == 3
cookie "sourcemeta_one_session[Max-Age]" == 0
cookie "sourcemeta_one_transaction[Max-Age]" == 0
cookie "sourcemeta_one_renewal[Max-Age]" == 0
header "Location" startsWith "https://keycloak:8443/realms/main/protocol/openid-connect/logout?"
header "Location" contains "id_token_hint="
header "Location" contains "post_logout_redirect_uri=http%3A%2F%2Flocalhost%3A8000"
Expand Down
Loading
Loading