Skip to content
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.

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

Fix: Server throws "User Does not have permission to access this" occasionally (arrow flight endpoint) #847

Merged
merged 8 commits into from
Jul 11, 2024

Conversation

dharanad
Copy link
Contributor

@dharanad dharanad commented Jul 9, 2024

WIP

Fixes #822

Description

When the parsable instance starts, it creates a session key for the admin user (of type SessionKey::BasicAuth) and adds it to the active session. This allows querying via Arrow Flight, as the session key is recognized as active.

However, when the admin user logs in, the admin session key (of type SessionKey::BasicAuth) is removed from the active session and new admin session of type SessionKey::SessionId is added. Consequently, any subsequent queries by clients using Arrow Flight fail the authentication step because the handler create a session key of type SessionKey::BasicAuth and tries to lookup in the map which fails for obvious reasons.

To address this, override permissions if the session key belongs to the user. This ensures that user sessions are maintained even if the admin logs in or logs out, preventing disruptions in client access via Arrow Flight.

Other Approach

  • To maintain a mapping for SessionKey::BasicAuth to SessionKey::SessionId. If map get operation fails for type BasicAuth we retrived the SessionId if exists and check if session id exists in active sessions.
  • Implement a hash function which yeilds same hash value for BasicAuth and SessionId if they belong to same user.

This PR has:

  • been tested to ensure log ingestion and log query works.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added documentation for new or modified features or behaviors.

Copy link
Contributor

github-actions bot commented Jul 9, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@dharanad
Copy link
Contributor Author

dharanad commented Jul 9, 2024

I have read the CLA Document and I hereby sign the CLA

nitisht added a commit to parseablehq/.github that referenced this pull request Jul 9, 2024
server/src/rbac.rs Outdated Show resolved Hide resolved
@dharanad dharanad marked this pull request as ready for review July 9, 2024 15:47
@nitisht
Copy link
Member

nitisht commented Jul 11, 2024

@nikhilsinhaparseable PTAL

Copy link
Contributor

@nikhilsinhaparseable nikhilsinhaparseable left a comment

Choose a reason for hiding this comment

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

LGTM

@nikhilsinhaparseable nikhilsinhaparseable merged commit 5133d1b into parseablehq:main Jul 11, 2024
8 checks passed
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.

Server throws "User Does not have permission to access this" occasionally (arrow flight endpoint).
3 participants