Correctly fallback to OIDC nested policy paths#1137
Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
🤖 Augment PR SummarySummary: This PR makes OIDC logins fall back to the first path governed by the matched policy when no valid Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
| @@ -81,14 +81,16 @@ auto encode_oidc_metadata(const std::string_view issuer, | |||
| const std::string_view client_id, | |||
There was a problem hiding this comment.
enterprise/authentication/authentication_save.cc:81 The comment immediately above encode_oidc_metadata describes the encoded fields but doesn’t mention the newly-serialized default_path, so the documented metadata layout is now incomplete.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| // taken as-is, and only a same-origin local path is honoured, so the login | ||
| // cannot be turned into an open redirect. Anything else falls back to what | ||
| // the policy governs. It is sealed into the transaction so that the | ||
| // callback trusts it |
There was a problem hiding this comment.
enterprise/server/include/sourcemeta/one/enterprise_server_action_auth_login_v1.h:141 This comment says the callback “trusts” the sealed to, but the callback action still re-validates it with is_local_path before redirecting; the wording may mislead readers about the defense-in-depth behavior.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Benchmark Index (community)
Details
| Benchmark suite | Current: 438959b | Previous: 37bc023 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
327 ms |
303 ms |
1.08 |
Add one schema (100 existing) |
28 ms |
30 ms |
0.93 |
Add one schema (1000 existing) |
86 ms |
95 ms |
0.91 |
Add one schema (10000 existing) |
732 ms |
791 ms |
0.93 |
Update one schema (1 existing) |
21 ms |
21 ms |
1 |
Update one schema (101 existing) |
28 ms |
31 ms |
0.90 |
Update one schema (1001 existing) |
90 ms |
95 ms |
0.95 |
Update one schema (10001 existing) |
735 ms |
792 ms |
0.93 |
Cached rebuild (1 existing) |
7 ms |
8 ms |
0.88 |
Cached rebuild (101 existing) |
9 ms |
10 ms |
0.90 |
Cached rebuild (1001 existing) |
30 ms |
35 ms |
0.86 |
Cached rebuild (10001 existing) |
258 ms |
288 ms |
0.90 |
Index 100 schemas |
649 ms |
618 ms |
1.05 |
Index 1000 schemas |
1325 ms |
1303 ms |
1.02 |
Index 10000 schemas |
13840 ms |
13420 ms |
1.03 |
Index 10000 schemas (custom meta-schema) |
16717 ms |
15970 ms |
1.05 |
Index 10000 schemas ($ref fan-out) |
17003 ms |
16138 ms |
1.05 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Benchmark Index (enterprise)
Details
| Benchmark suite | Current: 438959b | Previous: 37bc023 | Ratio |
|---|---|---|---|
Add one schema (0 existing) |
402 ms |
401 ms |
1.00 |
Add one schema (100 existing) |
115 ms |
105 ms |
1.10 |
Add one schema (1000 existing) |
178 ms |
154 ms |
1.16 |
Add one schema (10000 existing) |
918 ms |
746 ms |
1.23 |
Update one schema (1 existing) |
107 ms |
95 ms |
1.13 |
Update one schema (101 existing) |
112 ms |
100 ms |
1.12 |
Update one schema (1001 existing) |
177 ms |
154 ms |
1.15 |
Update one schema (10001 existing) |
879 ms |
745 ms |
1.18 |
Cached rebuild (1 existing) |
9 ms |
8 ms |
1.13 |
Cached rebuild (101 existing) |
12 ms |
10 ms |
1.20 |
Cached rebuild (1001 existing) |
37 ms |
30 ms |
1.23 |
Cached rebuild (10001 existing) |
295 ms |
251 ms |
1.18 |
Index 100 schemas |
493 ms |
761 ms |
0.65 |
Index 1000 schemas |
1570 ms |
1450 ms |
1.08 |
Index 10000 schemas |
14068 ms |
13553 ms |
1.04 |
Index 10000 schemas (custom meta-schema) |
17140 ms |
16366 ms |
1.05 |
Index 10000 schemas ($ref fan-out) |
17251 ms |
16391 ms |
1.05 |
This comment was automatically generated by workflow using github-action-benchmark.
Signed-off-by: Juan Cruz Viotti jv@jviotti.com