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.

Sign up for GitHub

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

Jump to bottom

MSAL for On-Behalf-Of token requests #372

Merged
merged 3 commits into from
May 5, 2022
Merged

MSAL for On-Behalf-Of token requests #372

merged 3 commits into from
May 5, 2022

Conversation

kostrse
Copy link
Contributor

@kostrse kostrse commented Apr 29, 2022

Direct implementation of token HTTP request was replaced by implementation from MSAL for Go.

This is consistent with client secret token requests in Azure SDK (here and here)

@kostrse kostrse requested a review from a team as a code owner April 29, 2022 18:39
@kostrse kostrse requested review from kevinwcyu and vickyyyyyyy and removed request for a team April 29, 2022 18:39
@kostrse kostrse mentioned this pull request Apr 29, 2022
Closed
10 tasks
andresmgot
andresmgot reviewed May 3, 2022
View reviewed changes
Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

Code looks good to me! (I tried and is also working for my setup). I just have a question about some metric collection that was going on there.

pkg/azuredx/azureauth/auth.go Show resolved Hide resolved
sunker
sunker reviewed May 4, 2022
View reviewed changes
pkg/azuredx/azureauth/aad_client.go Show resolved Hide resolved
pkg/azuredx/azureauth/aad_client.go Show resolved Hide resolved
go.mod Show resolved Hide resolved
andresmgot
andresmgot approved these changes May 5, 2022
View reviewed changes
Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@andresmgot andresmgot merged commit 75bae4c into grafana:main May 5, 2022
@kostrse kostrse deleted the obo-msal branch May 6, 2022 03:17
@yaelleC yaelleC mentioned this pull request May 17, 2022
Closed
@andresmgot andresmgot mentioned this pull request May 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sunker sunker sunker left review comments

@andresmgot andresmgot andresmgot approved these changes

@kevinwcyu kevinwcyu Awaiting requested review from kevinwcyu kevinwcyu was automatically assigned from grafana/aws-datasources

@vickyyyyyyy vickyyyyyyy Awaiting requested review from vickyyyyyyy vickyyyyyyy was automatically assigned from grafana/aws-datasources

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kostrse @sunker @andresmgot