-
-
Notifications
You must be signed in to change notification settings - Fork 610
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
Improve how we create new authorizations #7643
Conversation
840f988
to
26f18db
Compare
26f18db
to
469a47a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These look like excellent improvements. I'd like a small doc comment change and some consideration on another comment.
To ensure there are no deployability problems with this change, I have run the Go subset of our integration tests with both a new RA / old SA combo, and an old RA / new SA combo. Note the BuildIDs in the startup version logs, indicating the branch that the service was built from:
For posterity, the steps I followed to get this working were:
There has to be an easier way, but this definitely worked. |
Within the NewOrderAndAuthzsRequest, replace the corepb.Authorization field with a new sapb.NewAuthzRequest message. This message has all of the same field types and numbers, and the RA still populates all of these fields when constructing a request, for backwards compatibility. But it also has new fields (an Identifier carrying both type and value, a list of challenge types, and a challenge token) which the RA preferentially consumes if present.
This causes the content of our NewOrderAndAuthzsRequest to more closely match the content that will be created at the database layer. Although this may seem like a step backwards in terms of abstraction, it is also a step forwards in terms of both efficiency (not having to transmit multiple nearly-identical challenge objects) and correctness (being guaranteed that the token is actually identical across all challenges).
After this change is deployed, it will be followed by a change which removes the old fields from the NewAuthzRequest message, to realize the efficiency gains.
Part of #5913