Skip to content
This repository was archived by the owner on Jun 23, 2023. It is now read-only.

Dpop2 #56

Closed
wants to merge 6 commits into from
Closed

Dpop2 #56

wants to merge 6 commits into from

Conversation

rohe
Copy link
Collaborator

@rohe rohe commented May 20, 2021

Worked through the DPoP support.

OAuth 2.0 Demonstrating Proof-of-Possession at the Application Layer (DPoP)
draft-ietf-oauth-dpop-03

@rohe rohe requested review from nsklikas and peppelinux May 20, 2021 07:38
@peppelinux
Copy link
Member

sounds great Roland, can you fix the conflict in src/oidcop/oauth2/device_authorization.py?

We should improve this before merge: #54
this will increase about 2% but please take a look at the code, it can't be merged as it is, do your revision and tell me how to make change or merge and then do your commits, you drive.

overall, this PR looks awesome, ready for merge but: conflicts first.
I can do that but I think that you're already in the flow with this

peppelinux
peppelinux reviewed May 20, 2021
View reviewed changes
@peppelinux
Copy link
Member

peppelinux commented May 22, 2021 •
edited
Loading

I think that's ready to be merged, @nsklikas do you agree?

@peppelinux peppelinux self-requested a review May 22, 2021 13:33
@peppelinux
Copy link
Member

peppelinux commented May 22, 2021 •
edited
Loading

@rohe maybe some unit tests and usage examples can be done if you agree :)

@peppelinux peppelinux mentioned this pull request May 23, 2021
Merged
peppelinux
peppelinux suggested changes May 24, 2021
View reviewed changes
src/oidcop/oauth2/add_on/dpop.py
return request


def token_args(endpoint_context, client_id, token_args: Optional[dict] = None):
Copy link
Member

Choose a reason for hiding this comment

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

token_args is at the same time the name of the function and the name of an argument.
We should avoid naming collision, do you agree?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep!

@rohe rohe closed this May 24, 2021
@rohe
Copy link
Collaborator Author

rohe commented May 24, 2021

Included in #65

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@nsklikas nsklikas Awaiting requested review from nsklikas

1 more reviewer

@peppelinux peppelinux peppelinux requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rohe @peppelinux