This repository has been archived by the owner. It is now read-only.

make sure audience captures full origin #82

Closed
benadida opened this Issue Jul 19, 2011 · 3 comments

Comments

Projects
None yet
3 participants
@benadida
Contributor

benadida commented Jul 19, 2011

scheme and port, in particular.

@ghost ghost assigned lloyd Sep 30, 2011

@lloyd

This comment has been minimized.

Show comment
Hide comment
@lloyd

lloyd Oct 20, 2011

Contributor

ok, port is already in there, but it seems like we'll need to make sure we're normalizing (so that foo.com:80 and foo.com match).

I wrote a little library to do that once upon a time: https://github.com/lloyd/urlmatch.js - but I think that's overkill, because we can just use node's libs (normalization is work to be done inside the verifier): http://nodejs.org/docs/v0.4.12/api/url.html

More interesting to me is breaking our external contracts. Here's my proposal:

  1. change our code so that origin inside assertions includes scheme://host:port
  2. change the verifier so that it handles either a partial origin or a full origin inside the POST request.
  3. blog about the migration to full origins
  4. wait
  5. change the verifier to require scheme

sound sane?

Contributor

lloyd commented Oct 20, 2011

ok, port is already in there, but it seems like we'll need to make sure we're normalizing (so that foo.com:80 and foo.com match).

I wrote a little library to do that once upon a time: https://github.com/lloyd/urlmatch.js - but I think that's overkill, because we can just use node's libs (normalization is work to be done inside the verifier): http://nodejs.org/docs/v0.4.12/api/url.html

More interesting to me is breaking our external contracts. Here's my proposal:

  1. change our code so that origin inside assertions includes scheme://host:port
  2. change the verifier so that it handles either a partial origin or a full origin inside the POST request.
  3. blog about the migration to full origins
  4. wait
  5. change the verifier to require scheme

sound sane?

@benadida

This comment has been minimized.

Show comment
Hide comment
@benadida

benadida Oct 20, 2011

Contributor

the plan sounds very good.

Contributor

benadida commented Oct 20, 2011

the plan sounds very good.

lloyd added a commit that referenced this issue Oct 20, 2011

include full origin (inc. scheme) in generated assertions. To preserv…
…e compatibility, allow the rp to omit scheme in post data, and subsequent to verification, return whatever the rp sent us. issue #82

shane-tomlinson added a commit that referenced this issue Oct 20, 2011

Adding "getHostname" to user, which filters out the port and protocol.
Add's to Lloyd's pull request by keeping the extra info from being displayed to the user.

issue #82

@lloyd lloyd closed this in e20fca0 Oct 20, 2011

@jbonacci

This comment has been minimized.

Show comment
Hide comment
@jbonacci

jbonacci Oct 26, 2011

Contributor

Nothing specific for QA to test.

Contributor

jbonacci commented Oct 26, 2011

Nothing specific for QA to test.

lloyd added a commit that referenced this issue Nov 2, 2011

lloyd added a commit that referenced this issue Nov 2, 2011

lloyd added a commit that referenced this issue Nov 2, 2011

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