oidc visibility mod #1377
oidc visibility mod #1377
Conversation
WalkthroughThe changes make several functions in the OIDC handler file public and expose the OIDC module itself via the library root. No internal logic or control flow is altered; only visibility is updated to allow external access to these functions and the module. Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
🔭 Outside diff range comments (6)
src/handlers/http/oidc.rs (6)
261-272: Add documentation for the newly public function.Since this function is now part of the public API, it should have proper documentation explaining its purpose, parameters, and return value.
+/// Creates an HTTP redirect response to the specified URL with optional cookies. +/// +/// # Arguments +/// * `url` - The destination URL for the redirect +/// * `cookies` - Iterator of cookies to include in the response +/// +/// # Returns +/// Returns an HTTP 301 (Moved Permanently) response with the redirect location and cookies pub fn redirect_to_client( url: &str, cookies: impl IntoIterator<Item = Cookie<'static>>, ) -> HttpResponse {
282-289: Add documentation for the newly public function.Since this function is now part of the public API, it should have proper documentation explaining its purpose and usage.
+/// Creates a session cookie with the specified ULID. +/// +/// # Arguments +/// * `id` - The ULID to use as the session identifier +/// +/// # Returns +/// Returns a configured session cookie with appropriate security settings pub fn cookie_session(id: Ulid) -> Cookie<'static> {
291-298: Add documentation for the newly public function.Since this function is now part of the public API, it should have proper documentation explaining its purpose and usage.
+/// Creates a username cookie with the specified username. +/// +/// # Arguments +/// * `username` - The username to store in the cookie +/// +/// # Returns +/// Returns a configured username cookie with appropriate security settings pub fn cookie_username(username: &str) -> Cookie<'static> {
319-340: Add documentation and consider consistency with existing user management APIs.This function directly manipulates user metadata and cache. Since there's already a
put_userfunction in the rbac module, ensure consistency between these APIs.+/// Creates or updates a user in both metadata storage and local cache. +/// +/// # Arguments +/// * `username` - The username for the user +/// * `group` - Set of roles/groups for the user +/// * `user_info` - User information from OIDC provider +/// +/// # Returns +/// Returns the created/updated User or an ObjectStorageError +/// +/// # Errors +/// Returns ObjectStorageError if metadata operations fail pub async fn put_user(
342-373: Add documentation for the newly public function.Since this function is now part of the public API, it should have proper documentation explaining its purpose, parameters, and behavior.
+/// Updates a user's roles and information if they have changed. +/// +/// # Arguments +/// * `user` - The existing user to potentially update +/// * `group` - New set of roles/groups +/// * `user_info` - New user information from OIDC provider +/// +/// # Returns +/// Returns the updated User (or unchanged if no updates needed) or an ObjectStorageError +/// +/// # Errors +/// Returns ObjectStorageError if metadata operations fail pub async fn update_user_if_changed(
300-315: Reduce visibility of internal OIDC token handlerThis helper is only ever called within
src/handlers/http/oidc.rsand doesn’t need to be part of the public API surface. Making it private prevents accidental external use of core token-handling logic.• File: src/handlers/http/oidc.rs
Change the function signature from public to private:- pub async fn request_token( + async fn request_token( oidc_client: Arc<DiscoveredClient>, login_query: &Login, ) -> anyhow::Result<(Claims, Userinfo)> { … }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/handlers/http/oidc.rs(6 hunks)src/lib.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: de-sh
PR: parseablehq/parseable#0
File: :0-0
Timestamp: 2025-03-20T15:50:45.435Z
Learning: Pay close attention to code comments for typos and semantic clarity during reviews for the Parseable project.
🧬 Code Graph Analysis (1)
src/handlers/http/oidc.rs (2)
src/rbac/user.rs (1)
username(80-88)src/rbac/mod.rs (1)
put_user(52-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
Fixes #XXXX.
Description
This PR has:
Summary by CodeRabbit