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

Populate names in device flow status API response #3533

Merged
merged 2 commits into from
Nov 15, 2022

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Oct 31, 2022

Summary

This PR adds the user name and organization name to the device flow login response, so that all the fields in the response are correctly populated.

Previously the GetAccessKey operation did not populate the IssusedForName. This PR fixes the function to populate that field.

Also adds a lookup of the organization from the AccessKey.OrganizationID, so that we can include the name in the response.

@dnephin dnephin requested a review from ssoroka October 31, 2022 21:53
@dnephin dnephin changed the title Fix device flow status response Populate names in device flow status API response Oct 31, 2022
@@ -20,12 +20,18 @@ func FindDeviceFlowAuthRequest(ctx RequestContext, deviceCode string) (*models.D
return nil, err
}

// TODO: do this in the data function
// TODO: do these lookups in the data function when data.GetDeviceFlowAuthRequest
// is converted to SQL
if dfar.AccessKeyID > 0 {
dfar.AccessKey, err = data.GetAccessKey(ctx.DBTxn, data.GetAccessKeysOptions{ByID: dfar.AccessKeyID})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not part of this change but it seems a bit odd that the DeviceFlowAuthRequest has a separate AccessKeyID and AccessKey

Base automatically changed from dnephin/device-flow-unused to main November 1, 2022 14:44
@dnephin dnephin force-pushed the dnephin/device-flow-names-in-resp branch from 8dc124e to 93c71f3 Compare November 8, 2022 22:50
if dfar.AccessKey != nil {
if dfar.AccessKeyID != 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a bug. AccessKey is always nil because we don't populate it in the query used above.

@dnephin dnephin force-pushed the dnephin/device-flow-names-in-resp branch from ad46d2f to a34b12f Compare November 10, 2022 20:31
Populate user name and organization name in the login response
The accessKey field is always null, because we don't populate it in this call.
@dnephin dnephin force-pushed the dnephin/device-flow-names-in-resp branch from a34b12f to 82caed0 Compare November 15, 2022 23:26
@dnephin dnephin merged commit db22f1e into main Nov 15, 2022
@dnephin dnephin deleted the dnephin/device-flow-names-in-resp branch November 15, 2022 23:37
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.

2 participants