Skip to content

Conversation

@drode1
Copy link

@drode1 drode1 commented Jan 15, 2026

Description

This PR add support for OIDC Group verification during the authentication process for the PocketID provider.

Currently, access is restricted to users whose emails are explicitly listed in the panel or those who have a custom claim (remnawaveClaim). This change allows for a more standard OIDC flow by restricting access based on the groups claim provided in the ID token.

Frontend

Need to add new field + logic (what should be filled or field with mail/group) + I18N to support new fields.

Example configuration

Image

@snyk-io
Copy link

snyk-io bot commented Jan 15, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@what-the-diff
Copy link

what-the-diff bot commented Jan 15, 2026

PR Summary

  • New 'AllowedGroups' Field Added
    A new category named 'AllowedGroups' is now part of the OAuth2 settings. This category will be of list type, containing names of usergroups.

  • Configuration Initialization updated
    The above mentioned 'AllowedGroups' category has been successfully added to the initial configuration seeding process as an empty list.

  • Enhanced Access Control through Conditional Authorization
    The Authentication Service now has refined user access control. It includes the 'AllowedGroups' while constructing the user authorization URL, facilitating conditional access based on group memberships.

  • Validation Logic for User Group Matching
    A validation mechanism is now in place in the Authentication Service to cross-verify if a user belongs to an allowed group. This adds another layer of access control based on group memberships.

  • Updated Error Messaging
    The error messages have been updated to cover checks for both user email validation as well as group membership requirements.

@greptile-apps
Copy link

greptile-apps bot commented Jan 15, 2026

Greptile Summary

Added OIDC groups verification for PocketID authentication. The schema and seed files correctly add the allowedGroups field, but the implementation has a critical bug that prevents the feature from working.

Critical Issues:

  • Undefined variable pocketIdSettings used in three locations (lines 450, 782, 789) in auth.service.ts - this will cause runtime ReferenceError exceptions
  • Should be remnawaveSettings.oauth2Settings.pocketid instead

Implementation Logic:

  • Conditionally adds 'groups' scope to OAuth2 authorization if allowedGroups is configured
  • Checks user's group membership after email and custom claim checks
  • Falls back to deny if none of the checks pass (custom claim → allowed emails → allowed groups)

Confidence Score: 0/5

  • This PR has critical bugs that will cause runtime failures and prevent the feature from working
  • The code references an undefined variable pocketIdSettings in three locations, which will throw ReferenceError exceptions when PocketID authentication is attempted with group-based access control. This is a blocking issue that makes the feature completely non-functional.
  • src/modules/auth/auth.service.ts requires immediate attention - all three references to pocketIdSettings must be changed to remnawaveSettings.oauth2Settings.pocketid

Important Files Changed

Filename Overview
libs/contract/models/remnawave-settings/oauth2-settings.schema.ts Added allowedGroups field to PocketID OAuth2 settings schema
prisma/seed/config.seed.ts Added default empty allowedGroups array to PocketID settings in seed
src/modules/auth/auth.service.ts Critical bug - undefined pocketIdSettings variable causes runtime error

Sequence Diagram

sequenceDiagram
    participant User
    participant Frontend
    participant AuthService
    participant PocketID
    participant Database

    User->>Frontend: Click PocketID login
    Frontend->>AuthService: oauth2Authorize(POCKETID)
    AuthService->>Database: Get remnawave settings
    Database-->>AuthService: Settings with allowedGroups
    
    alt allowedGroups configured
        AuthService->>AuthService: Add 'groups' to scopes
    end
    
    AuthService->>PocketID: Redirect to authorize with scopes
    PocketID->>User: Show consent screen
    User->>PocketID: Approve
    PocketID-->>Frontend: Redirect with code
    
    Frontend->>AuthService: oauth2Callback(code)
    AuthService->>PocketID: Exchange code for tokens
    PocketID-->>AuthService: ID token with claims
    
    AuthService->>AuthService: Decode ID token
    AuthService->>AuthService: Check remnawaveClaim
    
    alt remnawaveClaim exists
        AuthService-->>Frontend: Success + JWT
    else Check allowed emails
        AuthService->>AuthService: Check if email in allowedEmails
        alt email allowed
            AuthService-->>Frontend: Success + JWT
        else Check allowed groups
            AuthService->>AuthService: Extract groups from claims
            AuthService->>AuthService: Check if any group in allowedGroups
            alt group allowed
                AuthService-->>Frontend: Success + JWT
            else No match
                AuthService-->>Frontend: Forbidden
            end
        end
    end
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

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