-
Notifications
You must be signed in to change notification settings - Fork 274
Fix some provisioner and policy prompt issues #1391
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
Conversation
1f417fb
to
bd5cdf3
Compare
bd5cdf3
to
4f46e13
Compare
This PR fixes the following issues: - SCEP provisioners not detected in admin token flows - Invalid provisioner selection logic when managing provisioner policies - Unexpected error messages showing "issuer" instead or "provisioner" flag
Long ago the "issuer" flag was used to denote what we not call provisioners. There were still some uses of `issuer` in the code, which have now been renamed to reflect their current usage. Only when the actual token is going to be signed, will it be called an `issuer` again.
4f46e13
to
064866f
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.
It looks good, but I would try to see if doing this would work:
ctx.Set("provisioner", "")
// and perhaps
// ctx.Set("issuer", "")
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.
It looks a bit hacky, but it would be nice to find a different way to set the flag to ""
for just the things we need. Perhaps with token options instead of getting things from the context in this "flow" methods.
418c526
to
c153ef3
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.
Looks good, but I would also set the alias to ""
. This will also fix the issue if the alias is in an environment variable or on defaults.json
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.
lgtm
This PR fixes the following issues:
--provisioner
flag was used to select a provisioner to authenticate as well as the provisioner to manage policies for.--issuer
flag value, whereas it was actually supplied in the--provisioner
flag. This fixes step cli flags with aliases sometimes report a different alias in error messages #821.issuer
in our code to reflect provisioner names.