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

Revert to old-style data formats #103

Merged
merged 1 commit into from Nov 6, 2014
Merged

Revert to old-style data formats #103

merged 1 commit into from Nov 6, 2014

Conversation

rfk
Copy link
Contributor

@rfk rfk commented Oct 31, 2014

Here's my attempt at #97, removing the new-style JWT-compatible formats and reverting to the old-style format as the only option.

A rough list of the changes made to accomplish this:

  • Revert "kty" field to "algorithm"
  • Revert "pubkey" field to "public-key"
  • Revert "sub" field to "principal"
  • Revert "DSA" to "DS" and "RSA" to "RS", removing notion of algorithm name aliases
  • Revert all timestamps to milliseconds rather than seconds
  • Revert b64-encoded field values to previous decimal or hex encoding
  • Remove "newFormat" and "version" switches in code and docs

@callahad what have I missed? :-)

I will test this with mozilla/fxa-auth-server#831 and a full fxa+sync stack deploy and report back.

@callahad
Copy link
Contributor

callahad commented Nov 5, 2014

Nothing springs to mind that you've missed, but it's been a long time since I've dug into the old data formats.

If tests pass, and you can parse assertions from production Persona, then you're probably fine.

@rfk
Copy link
Contributor Author

rfk commented Nov 5, 2014

Thanks Dan. So, things I'll sanity-check before we merge this:

  • that it works with fxa as currently deployed
  • that it can parse and verify assertions from production persona, and vice-versa
  • basic interoperability with PyBrowserID

If we can tick all those boxes we should be in good shape.

This removes a bunch of work on "new format" certificates and
assertions, which was designed to bring things into closer compliance
with the JWT spec but was ultimately never deployed.  It reduces
complexity and maintenance burden by remove formats that will not be
seen in the wild.
@rfk
Copy link
Contributor Author

rfk commented Nov 5, 2014

I've successfully tested the following combinations:

All seem to pass OK, so I think this is about as good as it'll get. @fmarier please r? for an extra set of eyes.

@callahad
Copy link
Contributor

callahad commented Nov 5, 2014

Looks good -- the assertions really don't change, so as long as you're parsing / generating an RS and a DS key correctly, it should be set.

@fmarier
Copy link
Contributor

fmarier commented Nov 6, 2014

Nice cleanup! r+
ilike

rfk added a commit that referenced this pull request Nov 6, 2014
Revert to old-style data formats as the only support format
@rfk rfk merged commit b81d3d5 into master Nov 6, 2014
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

3 participants