Skip to content

feat: OIDC authz providers#161

Open
dvaneson wants to merge 2 commits into
klarna-incubator:mainfrom
dvaneson:oidc-authz-providers
Open

feat: OIDC authz providers#161
dvaneson wants to merge 2 commits into
klarna-incubator:mainfrom
dvaneson:oidc-authz-providers

Conversation

@dvaneson

Copy link
Copy Markdown

Overview

We needed a way to support Authorization for Gram, but since we don't use LDAP there were no suitable providers aside from the static providers. So, I added providers to the OIDC plugin to support authorization based on group claims, after authenticating with the identity provider. These were based loosely on the existing LDAP providers.

This is my first time working with OIDC flows and using this web framework, so if there's anything that could be improved or done a different way please let me know.

Changes

  • OIDCIdentityProvider -> Already existed, but pulled out into it's own file. Pushes user info and group mappings to the DB after login
  • OIDCGroupBasedAuthzProvider -> Maps groups from claims to roles, based on provided mapping
  • OIDCGroupBasedReviewerProvider -> Determine users that are reviewers based on group membership
  • OIDCTeamProvider -> Convert groups to teams. Use filtering to only include a subset of the groups as valid teams
    • Also works as a SearchProvider to look up teams by name
  • OIDCUserProvider -> Convert user info to User object
  • OIDCUserStore -> For interacting with the gram-oidc DB, which stores information about users and their group mappings
    • oidc_users table for storing users
    • oidc_user_groups for storing user to group mappings

Went with a DB approach for storing group claims for a couple of reasons:

  • Didn't want to store and pass around the auth token for each user
  • You get all the information needed from the first call to the userinfo endpoint, so might as well use it
  • With the OIDC flow you can't easily look up which users belong to X group, which is important for looking up reviewers and having the search functionality for teams

Other Small Changes

@CLAassistant

CLAassistant commented Jun 26, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Tethik Tethik self-assigned this Jun 26, 2025
@Tethik

Tethik commented Jun 26, 2025

Copy link
Copy Markdown
Collaborator

Will review during the week - fyi @tank1st99 we've been discussing these changes on Gram's Discord. We should probably align so this doesn't break Klarna's deployment.

The semgrep finding may be an actual vulnerability from what I'm reading, curious to test it (and then we should report it as such here on github)

@Tethik Tethik removed their assignment Aug 23, 2025
@Tethik

Tethik commented Aug 23, 2025

Copy link
Copy Markdown
Collaborator

Had a look earlier in the summer but will require more work than I have time/energy for. @tank1st99 you may want to investigate the vulnerability here.

Comment thread .tool-versions
@@ -0,0 +1 @@
nodejs 20.18.1 No newline at end of file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This pins Node 20.18.1, but the project targets a newer Node. The open-source repo declares engines.node: "20.x || 22.x || 24.x", and Klarna's internal mirror pins Node 24.15.0 via Volta. Aligning .tool-versions to 24.15.0 keeps asdf users working while matching both the engines range and the Volta pin (also adds the missing trailing newline):

Suggested change
nodejs 20.18.1
nodejs 24.15.0

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.

4 participants