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

refactor(core)!: update user scopes #1922

Merged
merged 3 commits into from
Sep 15, 2022

Conversation

gao-sun
Copy link
Member

@gao-sun gao-sun commented Sep 14, 2022

Summary

  • expose original HttpError instead of showing 500
  • make new directory anyway when user chooses not to load official connectors
  • add scope / claim constants
  • update ID Token and Userinfo Endpoint claims to reflect the new design

Testing

  • UT
  • proper claims returned in ID Token / Userinfo Endpoint

@linear
Copy link

linear bot commented Sep 14, 2022

LOG-4053 Core: Update Userinfo Endpoint

Current user info endpoint response:

{
    "sub":"SS3pNBWAgAoc",
    "id":"SS3pNBWAgAoc",
    "username":"foo",
    "primary_email":null,
    "primary_phone":null,
    "name":null,
    "avatar":null,
    "role_names":[
        "admin"
    ],
    "custom_data":{
        "adminConsolePreferences":{
            "language":"en",
            "appearanceMode":"dark"
        }
    },
    "identities":{
        "github":{
            "userId":"10594507",
            "details":{
                "id":"10594507",
                "name":"Foo",
                "email":"[email protected]",
                "avatar":"https://avatars.githubusercontent.com/u/10594507?v=4"
            }
        }
    },
    "last_sign_in_at":1661499869711,
    "application_id":"foo"
}

Update to

Add scope to claim mapping:

  • profile → name, picture, username, role_names
  • email → email, email_verified
  • phone → phone_number, phone_number_verified
  • custom_data → custom_data
  • identities → identities

DO NOT include the following properties in claims (both user info endpoint response and ID token):

  • id (i.e. same as sub)
  • last_sign_in_at
  • application_id

Map DB table properties to standard claims:

  • avatar → picture
  • primary_email → email
  • primary_phone → phone_number

ID token claims:

sub: string; // user id
name?: string; // full name
username?: string;
picture?: string; // avatar
role_names?: string[]; // roles
email?: string; // primary_email
email_verified?: boolean; // if email exists, it will be true
phone_number?: string; // primary_phone
phone_number_verified?: boolean; // if phone_number exists, it will be true
  • email, email_verified, phone_number, phone_number_verified will be only returned when the related scope is provided.
  • Since custom_data and identities may be very large, they are not appropriate to be included in ID token.
    • Even though scope contains custom_data , identities, ID token will not contain them.

User info response claims:

sub: string; // user id
name?: string; // full name
username?: string;
picture?: string; // avatar
role_names?: string[]; // roles
email?: string; // primary_email
email_verified?: boolean; // if email exists, it will be true
phone_number?: string; // primary_phone
phone_number_verified?: boolean; // if phone_number exists, it will be true

// userinfo additional claims (not included in id token)
custom_data?: unknown;
identities?: unknown; // social identities: mask inner email property (safer)

/* Other properties from user DB table */
// id: string; // user id,same as sub, remove
// last_sign_in_at?: number; // timestamp with the timezone when the user signed in last time
// application_id?: string;// application the user first signed in to

---

Reference:

@github-actions
Copy link

github-actions bot commented Sep 14, 2022

COMPARE TO master

Total Size Diff 📈 +8.77 KB

Diff by File
Name Diff
packages/console/src/App.tsx 📈 +73 Bytes
packages/console/src/components/AppContent/components/UserInfo/index.tsx 📈 +197 Bytes
packages/core/package.json 📈 +65 Bytes
packages/core/src/env-set/add-connectors.ts 📈 +68 Bytes
packages/core/src/middleware/koa-error-handler.test.ts 📈 +355 Bytes
packages/core/src/middleware/koa-error-handler.ts 📈 +137 Bytes
packages/core/src/oidc/init.ts 📈 +340 Bytes
packages/core/src/oidc/scope.test.ts 📈 +1.6 KB
packages/core/src/oidc/scope.ts 📈 +1.54 KB
packages/shared/src/index.ts 📈 +25 Bytes
packages/shared/src/scope.ts 📈 +2.45 KB
pnpm-lock.yaml 📈 +1.95 KB

@gao-sun gao-sun requested review from a team, charIeszhao and simeng-li and removed request for a team September 14, 2022 02:14
@github-actions github-actions bot added size/l and removed size/m labels Sep 14, 2022
@gao-sun gao-sun marked this pull request as ready for review September 14, 2022 02:14
packages/shared/src/scope.ts Outdated Show resolved Hide resolved
packages/shared/src/scope.ts Show resolved Hide resolved
packages/core/src/env-set/add-connectors.ts Show resolved Hide resolved
packages/shared/src/scope.ts Outdated Show resolved Hide resolved
packages/shared/src/scope.ts Show resolved Hide resolved
Copy link
Contributor

@wangsijie wangsijie left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@xiaoyijun xiaoyijun left a comment

Choose a reason for hiding this comment

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

LGTM

@gao-sun gao-sun merged commit 8d22b5c into master Sep 15, 2022
@gao-sun gao-sun deleted the gao-log-4053-core-update-userinfo-endpoint branch September 15, 2022 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Make it better size/l
Development

Successfully merging this pull request may close these issues.

7 participants