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 percent-encoding for identity user portion #81

Merged
merged 5 commits into from Nov 17, 2018

Conversation

adamroach
Copy link
Contributor

No description provided.

Copy link
Contributor

@ekr ekr left a comment

Choose a reason for hiding this comment

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

LGTM

ensuring that they are authoritative for the identity.
Sites that use identities that are not of the form
<user>@<domain>, or which use identities in
which the domain portion does not match the domain name of

Choose a reason for hiding this comment

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

I think this is a new requirement. It means that an IDP proxy of "identities-r-us.com" cannot serve identities of the form "user@federated-domain.com" without using the form "user%40federated-domain@identities-r-us.com".
It would be more aesthetically pleasing, at least to me, if the identity of the identity provider was carried in a different field than the identity of the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's already true, because the text immediately above this says that if the identity doesn't match the IdP proxy address, then it must be rejected. This text simply tells IdP proxies how not to have their identities rejected on that basis.

Copy link
Contributor

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

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

I think that this could be simplified, thereby avoiding some potential problems.

ensuring that they are authoritative for the identity.
Sites that use identities that are not of the form
<user>@<domain>, or which use identities in
which the domain portion does not match the domain name of
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not support Harald's observation (which would be reasonable if this were made 4 years ago), but would instead suggest that this simply say that if the user portion of the identity contains the characters '%' or '@', then those characters MUST be escaped [...].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started out trying to say that, but had difficult saying it in a sufficiently precise way. I'm happy to take edits if you think you can make a run at it, but I tried and wasn't satisfied with the result.

Copy link
Contributor

Choose a reason for hiding this comment

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

draft-ietf-rtcweb-security-arch.xml Outdated Show resolved Hide resolved
draft-ietf-rtcweb-security-arch.xml Outdated Show resolved Hide resolved
@seanturner
Copy link
Contributor

@ekr I think this one is good to merge.

@ekr ekr merged commit c682593 into rtcweb-wg:master Nov 17, 2018
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