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

Remove checks based on response_type #46

Merged
merged 1 commit into from
Jan 1, 2021

Conversation

Zegnat
Copy link
Collaborator

@Zegnat Zegnat commented Aug 11, 2020

IndieAuth has always had some weird handling of response_type. All responses were always of OAuth type code, but the standard defined its own type id as well as making the field optional.

In the latest version of the IndieAuth standard, these decisions have been revised in favour of sticking to OAuth’s more standard handling. IndieAuth only accepts response_type=code. See indieweb/indieauth#42.

For backwards compatibility, this change still accept both code and id, as well as an empty value. All checks about differences between code and id have been removed as it should all just be handled as if a client had used the value code.

@sebastiangreger
Copy link

I can confirm this PR to work; since https://github.com/aaronpk/indielogin.com got updated to the latest IndieAuth spec a week ago, it was no longer possible to log in to the IndieWeb wiki using Selfauth. Applying these edits by @Zegnat everything works smoothly again.

@fluffy-critter
Copy link

I just updated my selfauth to this branch as well, and it works for me. The code changes also look reasonable. Could this please be merged in?

@Zegnat
Copy link
Collaborator Author

Zegnat commented Dec 7, 2020

Not a big fan of approving my own PRs. But making a note now that if nobody has had a look at this yet during the week, I am merging it anyway. Seems enough people have tried it out that it seems like a solid enough solution.

@aaronpk
Copy link
Contributor

aaronpk commented Dec 7, 2020

I'd say go for it, especially because it's not clear who you're waiting for a review from 😄

Copy link

@fluffy-critter fluffy-critter left a comment

Choose a reason for hiding this comment

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

Still looks good to me, I don’t have write access though so it’s not like this review counts for anything to GitHub ;)

@Zegnat
Copy link
Collaborator Author

Zegnat commented Dec 23, 2020

I'd say go for it, especially because it's not clear who you're waiting for a review from smile

Anyone, really. This repository is specifically set up that someone with write access needs to review and it cannot be the person who opens the PR. I also do not own this repository, so not sure how I feel about taking full responsibility 😉

Guess I will need to check if I have push rights into the master branch and see if I can merge manually with git and push a branch if no people with write access look at this repo anymore.

Copy link
Collaborator

@sknebel sknebel left a comment

Choose a reason for hiding this comment

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

LGTM

@Zegnat Zegnat merged commit a76ca38 into Inklings-io:master Jan 1, 2021
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.

None yet

5 participants