Skip to content

Conversation

@grahamplata
Copy link
Member

@grahamplata grahamplata commented Oct 10, 2025

Adds backend support for adding custom attributes that propagate in a JWT

  • Proto definitions and endpoints
  • Database schema and migrations
  • Template function as_sql_list
  • JWT token integration
# Only allow access when restaurant_id user attribute is defined 
security: 
  access: {{ .user.restaurant_id }} IS NOT NULL
  row_filter: " restaurant_id = {{ .user.restaurant_id }}"

Example Policy ^

In another PR I'd like to add CLI commands for attribute management and bulk import / export

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch (I will once the rest of the requests are done)
  • I'm proud of this work!

@grahamplata grahamplata changed the title Gplata/plat 210 1 Custom user attributes support Oct 10, 2025
@grahamplata grahamplata self-assigned this Oct 10, 2025
@grahamplata grahamplata marked this pull request as ready for review October 10, 2025 18:45
Copy link
Contributor

@begelundmuller begelundmuller left a comment

Choose a reason for hiding this comment

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

It also needs CLI support for viewing attributes (probably in rill user list?) and updating attributes (maybe rill user set-attributes?)

@grahamplata
Copy link
Member Author

It also needs CLI support for viewing attributes (probably in rill user list?) and updating attributes (maybe rill user set-attributes?)

Happy to add it in this PR, I was going to just do it in a follow up

populate the default attributes if they are nil

support custom attributes overrides
Update the query to use a wildcard for user email matching.
Copy link
Contributor

@begelundmuller begelundmuller left a comment

Choose a reason for hiding this comment

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

Can you also add an integration test for this feature in admin/server/rbac_test.go? Unlike the Postgres DB tests, that tests at the API level and covers permission checks, etc.


// Find the organization member
members, err := s.admin.DB.FindOrganizationMemberUsers(ctx, org.ID, "", true, user.Email, 1, "")
members, err := s.admin.DB.FindOrganizationMemberUsers(ctx, org.ID, "", true, "", 1, user.Email)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not safe to rely on a search pattern for a single-row lookup. For example, emails can have a % in them. Maybe add a proper singular FindOrganizationMemberUser DB query?

Comment on lines 1384 to 1385
require.Equal(t, "123", resp.Member.Attributes.Fields["restaurant_id"].GetStringValue())
require.Equal(t, "engineering", resp.Member.Attributes.Fields["department"].GetStringValue())
Copy link
Contributor

Choose a reason for hiding this comment

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

Convert the proto types to Go types instead of using them directly. In this case, could it just do require.Equal(t, attrs, resp.Member.Attributes.AsMap())?

Comment on lines 1408 to 1410
require.Equal(t, "456", updatedResp.Member.Attributes.Fields["restaurant_id"].GetStringValue())
require.Equal(t, "platform", updatedResp.Member.Attributes.Fields["team"].GetStringValue())
require.False(t, updatedResp.Member.Attributes.Fields["department"] != nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question

@begelundmuller begelundmuller merged commit 55d9b28 into main Nov 11, 2025
15 checks passed
@begelundmuller begelundmuller deleted the gplata/plat-210-1 branch November 11, 2025 19:03
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.

3 participants