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

feat(wh): Add oidc related columns to wh_user_dimension #1455

Merged
merged 1 commit into from
Aug 11, 2021
Merged

Conversation

tmessi
Copy link
Member

@tmessi tmessi commented Aug 11, 2021

This adds new columns to the wh_user_dimension to provide additional
data from OIDC that does not apply to Password Auth.

Note: Given the specifics of the data, it is possible these columns may
be used by other auth methods in the future, hence the lack of oidc in
the column names.

The specific columns are:

  • auth_method_external_id
    For OIDC this is the Issuer
  • auth_account_external_id
    For OIDC this is the Subject
  • auth_account_full_name
    For OIDC this is the account's Full Name, and can be None.
  • auth_account_email
    For OIDC this is the account's Email, and can be None.

For all of these new columns, they will be Not Applicable if the auth
method is Password.

Comment on lines 22 to 23
coalesce(apa.name, 'None') as auth_account_name,
coalesce(apa.description, 'None') as auth_account_description,
Copy link
Member

Choose a reason for hiding this comment

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

I think the auth_oidc_account table also has name and description columns. If so, these need to be case statements.

Comment on lines 47 to 48
coalesce(apm.name, 'None') as auth_method_name,
coalesce(apm.description, 'None') as auth_method_description,
Copy link
Member

Choose a reason for hiding this comment

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

The auth_oidc_method table has name and description columns so these should be case statements too.

case
when apa.public_id is not null then 'Not Applicable'
when aoa.public_id is null then 'None'
else aoa.issuer
Copy link
Member

Choose a reason for hiding this comment

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

This should be the issuer from the auth_oidc_method table.

left join auth_method as am on aa.auth_method_id = am.public_id
left join auth_password_account as apa on aa.public_id = apa.public_id
left join auth_password_method as apm on am.public_id = apm.public_id
left join auth_oidc_account as aoa on aa.public_id = aoa.public_id
Copy link
Member

Choose a reason for hiding this comment

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

Need a join for auth_oidc_method

mgaffney
mgaffney previously approved these changes Aug 11, 2021
This adds new columns to the wh_user_dimension to provide additional
data from OIDC that does not apply to Password Auth.

Note: Given the specifics of the data, it is possible these columns may
be used by other auth methods in the future, hence the lack of `oidc` in
the column names.

The specific columns are:

- auth_method_external_id
    For OIDC this is the Issuer
- auth_account_external_id
    For OIDC this is the Subject
- auth_account_full_name
    For OIDC this is the account's Full Name, and can be `None`.
- auth_account_email
    For OIDC this is the account's Email, and can be `None`.

For all of these new columns, they will be `Not Applicable` if the auth
method is Password.
@tmessi tmessi merged commit 0f19009 into main Aug 11, 2021
@tmessi tmessi deleted the tmessi/wh-oidc branch August 11, 2021 22:45
@jefferai jefferai added this to the 0.5.1 milestone Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants