Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Include 'generation' field in signed certificates. #530

Merged
merged 1 commit into from Jan 29, 2014

Conversation

rfk
Copy link
Contributor

@rfk rfk commented Jan 29, 2014

Preliminary implementation of #486. This all needs to be revisited for new-format certificates, but this should be enough to get things working for now. @dannycoates r?

@rfk
Copy link
Contributor Author

rfk commented Jan 29, 2014

Hmm, @lloyd's example here suggests we should call this "fxa-generation":

https://github.com/mozilla/browserid-verifier#extra-idp-claims

Consensus?

@lloyd
Copy link
Contributor

lloyd commented Jan 29, 2014

I just made that up on the spot. I abstain from the vote here!

@ckarlof
Copy link
Contributor

ckarlof commented Jan 29, 2014

Hmm, @lloyd's example here suggests we should call this "fxa-generation":
https://github.com/mozilla/browserid-verifier#extra-idp-claims
Consensus?

@lloyd seems to be suggesting we prefix our IdP specific claims. I'm fine with it.

@@ -196,7 +197,8 @@ module.exports = function (
uid: emailRecord.uid,
email: emailRecord.email,
emailCode: emailRecord.emailCode,
emailVerified: emailRecord.emailVerified
emailVerified: emailRecord.emailVerified,
verifierSetAt: emailRecord.verifierSetAt
Copy link
Contributor

Choose a reason for hiding this comment

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

since we're adding verifierSetAt I think we should add it to the "getter" db.sessionToken too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, its already there. I'll look at this again when the 💊 has worn off.

@dannycoates
Copy link
Contributor

suggesting we prefix our IdP specific claims. I'm fine with it.

I don't care either way but namespacing seems safer in general.

@dannycoates
Copy link
Contributor

LGTM, r+ modulo whether we care to change the property name.

@rfk
Copy link
Contributor Author

rfk commented Jan 29, 2014

namespacing it is, will update and merge shortly

@rfk
Copy link
Contributor Author

rfk commented Jan 29, 2014

fxa-generation it is; merging

rfk added a commit that referenced this pull request Jan 29, 2014
Include 'fxa-generation' field in signed certificates.
@rfk rfk merged commit 49dd9de into master Jan 29, 2014
@shane-tomlinson shane-tomlinson deleted the rfk/generation-number-in-assertion branch April 18, 2018 12:50
rfk added a commit that referenced this pull request Oct 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants