Skip to content

Extend OIDC discovery parsing in preparation for SSO#1121

Merged
jviotti merged 1 commit into
mainfrom
authentication-discovery
Jul 14, 2026
Merged

Extend OIDC discovery parsing in preparation for SSO#1121
jviotti merged 1 commit into
mainfrom
authentication-discovery

Conversation

@jviotti

@jviotti jviotti commented Jul 14, 2026

Copy link
Copy Markdown
Member

Signed-off-by: Juan Cruz Viotti jv@jviotti.com

Review in cubic

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
@augmentcode

augmentcode Bot commented Jul 14, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR factors OpenID Connect (OIDC) discovery URL derivation and metadata parsing into a dedicated discovery module to support upcoming SSO work.

Changes:

  • Replaced ad-hoc discovery URL/JWKS parsing helpers in enterprise/authentication/authentication.cc with calls to discovery_url() and discovery_parse().
  • Added an enterprise implementation (enterprise/authentication/discovery.cc) that parses OIDC discovery JSON and extracts known endpoint fields.
  • Added a community-edition stub (src/authentication/discovery.cc) that returns no discovery information (matching the enterprise-only feature split).
  • Introduced a new public header authentication_discovery.h and wired it into authentication.h.
  • Extended the enterprise authentication unit test target to include new discovery parsing tests.

Technical Notes: Discovery parsing returns an optional document only when the body is a JSON object; individual endpoints are optional and are only set when present as JSON strings.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 9 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/authentication/include/sourcemeta/one/authentication.h">

<violation number="1" location="src/authentication/include/sourcemeta/one/authentication.h:8">
P2: The two new includes (`authentication_discovery.h` and `authentication_session.h`) are unused by the `Authentication` class interface. They add unnecessary compilation dependencies to this public header — any change to either file triggers a rebuild of every translation unit that includes `authentication.h`. Add them only when the class declaration actually references types or functions from these headers.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

#include <sourcemeta/one/authentication_export.h>
#endif

#include <sourcemeta/one/authentication_discovery.h>

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: The two new includes (authentication_discovery.h and authentication_session.h) are unused by the Authentication class interface. They add unnecessary compilation dependencies to this public header — any change to either file triggers a rebuild of every translation unit that includes authentication.h. Add them only when the class declaration actually references types or functions from these headers.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/authentication/include/sourcemeta/one/authentication.h, line 8:

<comment>The two new includes (`authentication_discovery.h` and `authentication_session.h`) are unused by the `Authentication` class interface. They add unnecessary compilation dependencies to this public header — any change to either file triggers a rebuild of every translation unit that includes `authentication.h`. Add them only when the class declaration actually references types or functions from these headers.</comment>

<file context>
@@ -5,7 +5,9 @@
 #include <sourcemeta/one/authentication_export.h>
 #endif
 
+#include <sourcemeta/one/authentication_discovery.h>
 #include <sourcemeta/one/authentication_error.h>
+#include <sourcemeta/one/authentication_session.h>
</file context>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Benchmark Index (community)

Details
Benchmark suite Current: 6cc40f1 Previous: ff62f9d Ratio
Add one schema (0 existing) 318 ms 351 ms 0.91
Add one schema (100 existing) 28 ms 32 ms 0.88
Add one schema (1000 existing) 86 ms 88 ms 0.98
Add one schema (10000 existing) 721 ms 759 ms 0.95
Update one schema (1 existing) 21 ms 21 ms 1
Update one schema (101 existing) 31 ms 30 ms 1.03
Update one schema (1001 existing) 87 ms 89 ms 0.98
Update one schema (10001 existing) 719 ms 764 ms 0.94
Cached rebuild (1 existing) 7 ms 7 ms 1
Cached rebuild (101 existing) 9 ms 9 ms 1
Cached rebuild (1001 existing) 29 ms 30 ms 0.97
Cached rebuild (10001 existing) 251 ms 256 ms 0.98
Index 100 schemas 565 ms 607 ms 0.93
Index 1000 schemas 1466 ms 1343 ms 1.09
Index 10000 schemas 13799 ms 14552 ms 0.95
Index 10000 schemas (custom meta-schema) 16525 ms 17327 ms 0.95
Index 10000 schemas ($ref fan-out) 16696 ms 16815 ms 0.99

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Benchmark Index (enterprise)

Details
Benchmark suite Current: 6cc40f1 Previous: ff62f9d Ratio
Add one schema (0 existing) 401 ms 388 ms 1.03
Add one schema (100 existing) 107 ms 113 ms 0.95
Add one schema (1000 existing) 164 ms 174 ms 0.94
Add one schema (10000 existing) 796 ms 854 ms 0.93
Update one schema (1 existing) 99 ms 106 ms 0.93
Update one schema (101 existing) 105 ms 113 ms 0.93
Update one schema (1001 existing) 162 ms 173 ms 0.94
Update one schema (10001 existing) 798 ms 858 ms 0.93
Cached rebuild (1 existing) 9 ms 9 ms 1
Cached rebuild (101 existing) 11 ms 11 ms 1
Cached rebuild (1001 existing) 32 ms 35 ms 0.91
Cached rebuild (10001 existing) 264 ms 282 ms 0.94
Index 100 schemas 688 ms 540 ms 1.27
Index 1000 schemas 1561 ms 1610 ms 0.97
Index 10000 schemas 14336 ms 13877 ms 1.03
Index 10000 schemas (custom meta-schema) 17278 ms 16470 ms 1.05
Index 10000 schemas ($ref fan-out) 17246 ms 16754 ms 1.03

This comment was automatically generated by workflow using github-action-benchmark.

@jviotti
jviotti merged commit d9c0b65 into main Jul 14, 2026
5 checks passed
@jviotti
jviotti deleted the authentication-discovery branch July 14, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant