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

Add the token back as an optional argument that is fetched dynamically. #45

Merged
merged 3 commits into from
Sep 20, 2024

Conversation

bvandersloot-mozilla
Copy link
Collaborator

Aiming to resolve #42

@bvandersloot-mozilla
Copy link
Collaborator Author

@ekovac: does this make sense in the context of the discussion on #42?

@ekovac
Copy link
Contributor

ekovac commented Sep 17, 2024

If the tokenURL has to be supplied by the RP, that feels like an arbitrary difference vs "full" FedCM. Is there any reason we don't want to just use the existing configURL mechanism?

@bvandersloot-mozilla
Copy link
Collaborator Author

I considered using the whole configURL mechanism. The biggest piece is that we don't need the protections offered by the .well-known resource's redirection, since this request is sent after the credential is chosen. Then it makes sense to me to make it as easy to use as possible, which is just another Javascript parameter. The well-known is still part of the opt-in path to FedCM, and the endpoint can stay the same.

I realized while writing this that a better place to put this is in the IDP's store, which is more equivalent to the configURL. Change made.

Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Human-facing, should be editorial.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <[email protected]>
@bvandersloot-mozilla bvandersloot-mozilla merged commit 30ff958 into main Sep 20, 2024
@bvandersloot-mozilla bvandersloot-mozilla deleted the bvandersloot-add-token branch September 20, 2024 13:14
@samuelgoto
Copy link
Contributor

Is tokenUrl the same as the id_assertion_endpoint?

@bvandersloot-mozilla
Copy link
Collaborator Author

Exactly! Maybe it would be better to rename it to idAssertionURL?

@samuelgoto
Copy link
Contributor

It is a little ugly, but I think conceptually right :)

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.

Information in the IdentityCredential on output
4 participants