Skip to content

Old issues #15

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

Merged
merged 3 commits into from
May 9, 2025
Merged

Old issues #15

merged 3 commits into from
May 9, 2025

Conversation

dickhardt
Copy link
Collaborator

dickhardt/openid-provider-commands#12

@aaronpk does this capture the meaning better?

@aaronpk
Copy link

aaronpk commented Apr 17, 2025

It looks like this PR also has the commit from #14 in it. I assume that's not intentional. It's not a huge deal here because it's just one commit, but it will become difficult to pick apart the diffs for larger commits in the future.

@aaronpk
Copy link

aaronpk commented Apr 17, 2025

I'm a bit confused about how the description of the Unauthorize command is split between two sections, I feel like that is not helping the clarity of what is expected here.

There's the "Unauthorize Functionality" section which says

The RP MUST revoke all active sessions and MUST reverse all authorization that may have been granted to applications, including offline_access, for Account resources identified by the sub.

That's pretty clear. But then there's the "Unauthorize Command" section which says

The OP sends this Command when it wants the RP to undo the last OpenID Connect login.

which is where the confusion is happening. It would probably be better to remove any description of the outcome from the command section, leaving the outcomes to be only described in the "Unauthorize Functionality" section.

@dickhardt
Copy link
Collaborator Author

It looks like this PR also has the commit from #14 in it. I assume that's not intentional. It's not a huge deal here because it's just one commit, but it will become difficult to pick apart the diffs for larger commits in the future.

That was not intentional -- indicator of my lack of GitHub skills

@dickhardt
Copy link
Collaborator Author

which is where the confusion is happening. It would probably be better to remove any description of the outcome from the command section, leaving the outcomes to be only described in the "Unauthorize Functionality" section.

I agree that would be more crisp. Will adjust.

@dickhardt dickhardt merged commit 0158d86 into main May 9, 2025
1 check passed
@dickhardt dickhardt deleted the old-issues branch May 9, 2025 15:47
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.

3 participants