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

Support app_tid #94

Merged
merged 15 commits into from
Oct 16, 2023
Merged

Support app_tid #94

merged 15 commits into from
Oct 16, 2023

Conversation

hebelal
Copy link
Contributor

@hebelal hebelal commented Sep 14, 2023

No description provided.

@hebelal hebelal requested a review from a team as a code owner September 14, 2023 10:27
f-blass
f-blass requested changes Sep 25, 2023
View reviewed changes
Copy link
Member

@f-blass f-blass left a comment •
edited
Loading

Choose a reason for hiding this comment

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

One potential issue and some cosmetic changes. Rest looks good from my side 👍

@liga-oz Could you please also provide a review for this PR?

oidcclient/jwk.go Outdated Show resolved Hide resolved
oidcclient/jwk_test.go Outdated Show resolved Hide resolved
auth/token.go Outdated Show resolved Hide resolved
liga-oz
liga-oz suggested changes Sep 26, 2023
View reviewed changes
Copy link
Contributor

@liga-oz liga-oz left a comment

Choose a reason for hiding this comment

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

Not sure if that is relevant for you here, but there is another change coming from the IAS server side that will introduce another header x-azp that will enable app2app and app2service calls. We have currently a PR open for it. You can check it out SAP/cloud-security-services-integration-library#1285

auth/token.go Outdated Show resolved Hide resolved
auth/token.go Outdated Show resolved Hide resolved
env/iasConfig.go Outdated Show resolved Hide resolved
oidcclient/jwk_test.go Outdated Show resolved Hide resolved
@hebelal
Copy link
Contributor Author

hebelal commented Oct 4, 2023

Added support for azp token claim

@hebelal hebelal requested review from f-blass and liga-oz October 4, 2023 12:30
f-blass
f-blass requested changes Oct 5, 2023
View reviewed changes
Copy link
Member

@f-blass f-blass left a comment

Choose a reason for hiding this comment

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

Sorry for another round.. I stumbled over a potential issue which was even there before.

oidcclient/jwk.go Outdated Show resolved Hide resolved
oidcclient/jwk.go Outdated Show resolved Hide resolved
liga-oz
liga-oz reviewed Oct 6, 2023
View reviewed changes
oidcclient/jwk.go Outdated Show resolved Hide resolved
@hebelal
Copy link
Contributor Author

hebelal commented Oct 6, 2023 •
edited
Loading

That might be a dump question, but doesn't the jwks result depend on which header we sent?
Because right now we only store one jwks result that every cached user gets.

@hebelal hebelal requested review from f-blass and liga-oz October 9, 2023 12:58
liga-oz
liga-oz approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@liga-oz liga-oz left a comment

Choose a reason for hiding this comment

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

lgtm

@hebelal hebelal merged commit b3b9553 into master Oct 16, 2023
9 checks passed
@hebelal hebelal deleted the app_tid branch October 16, 2023 08:07
This was referenced Oct 16, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liga-oz liga-oz liga-oz approved these changes

@f-blass f-blass f-blass approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@hebelal @f-blass @liga-oz @hebelsan