bugfix for oidc #1272
bugfix for oidc #1272
Conversation
WalkthroughThis pull request introduces two changes. A minor formatting update was applied in the Changes
Sequence Diagram(s)sequenceDiagram
participant QS as QueryServer:init
participant SS as QueryServer:start
participant PO as PARSEABLE.options
QS->>SS: Call start(shutdown_rx, prometheus, PO.openid())
SS->>PO: Retrieve OpenID configuration
SS-->>QS: Return initialization result
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/handlers/http/ingest.rs(1 hunks)src/handlers/http/modal/query_server.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: coverage
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
🔇 Additional comments (1)
src/handlers/http/ingest.rs (1)
92-92: Formatting improvement for better code readability.The addition of a blank line improves readability by separating the variable declaration from its subsequent usage.
| let result = self | ||
| .start(shutdown_rx, prometheus.clone(), PARSEABLE.options.openid()) | ||
| .await; |
There was a problem hiding this comment.
Fixed OIDC configuration not being passed to server.
This change correctly passes the OpenID Connect configuration from PARSEABLE.options.openid() to the start method, replacing the previously hardcoded None value. This properly initializes the authentication mechanism with the configured OIDC settings, which addresses the OIDC-related bug mentioned in the PR title.
There was a problem hiding this comment.
yes that was the issue, and was a simple fix, anything to add here?
There was a problem hiding this comment.
@nikhilsinhaparseable The fix looks solid! Since this was a straightforward issue where the OIDC configuration wasn't being properly passed to the server, the solution is appropriately simple and direct.
If you haven't already, it might be worth verifying that OIDC authentication works as expected after this change with a quick end-to-end test. Otherwise, this appears to be a complete fix for the issue.
🐰 nods in agreement
3 participants
Fixes #XXXX.
Description
This PR has:
Summary by CodeRabbit
These improvements enhance the system’s secure access mechanisms without altering its overall functionality, ensuring that your login and access remain smooth and reliable.