-
Notifications
You must be signed in to change notification settings - Fork 75
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
Add support for KRL #10
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for the pr, Pino! can you look at/sign the CLA? |
Oh, I thought I did. I'll check it again in a few hours.
|
I checked the CLA and I'm not able to edit or re-sign it- but it has all my information. |
the cla checker says you still haven't signed our cla. what do you see when you go to https://cla-assistant.io/uber/pam-ussh ? |
@pinodeca any luck re-signing the CLA? I can try to talk to our github admins if it's not letting you sign for some reason. |
Terribly sorry for dropping this thread. When I'm signed into GitHub and go to https://cla-assistant.io/uber/pam-ussh I see: That is followed by the license text, my personal information and the checked checkbox "By checking this box, I am electronically signing..." None of it is editable. |
Signed-off-by: Pino de Candia <giuseppe.decandia@gmail.com>
can you send me a screenshot of what you see when you go to the cla page? |
@pmoody- @gdecandia the signing isn't working here, due to the email mismatch. The diff uses the email "giuseppe.decandia@gmail.com" vs "giuseppedecanida@gmail.com" |
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.
can you add a test for this? if it's easier, you can pull the whole krlFile parsing into its own function and just test that.
return authenticate2(w, uid, username, ca, principals, "") | ||
} | ||
|
||
func authenticate2(w io.Writer, uid int, username, ca string, principals map[string]struct{}, krlFile string) AuthResult { |
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.
can you just call this authenticate
?
@@ -118,6 +124,22 @@ func authenticate(w io.Writer, uid int, username, ca string, principals map[stri | |||
return AuthError | |||
} | |||
|
|||
var parsedKRL *krl.KRL | |||
parsedKRL = nil | |||
if len(krlFile) > 0 { |
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.
nit, if krlFile != "" {
@@ -118,6 +124,22 @@ func authenticate(w io.Writer, uid int, username, ca string, principals map[stri | |||
return AuthError | |||
} | |||
|
|||
var parsedKRL *krl.KRL | |||
parsedKRL = nil |
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.
this isn't needed, it should be nil by default.
return authenticate2(w, uid, username, ca, principals, "") | ||
} | ||
|
||
func authenticate2(w io.Writer, uid int, username, ca string, principals map[string]struct{}, krlFile string) AuthResult { |
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.
can you just call this authenticate
?
@@ -118,6 +124,22 @@ func authenticate(w io.Writer, uid int, username, ca string, principals map[stri | |||
return AuthError | |||
} | |||
|
|||
var parsedKRL *krl.KRL | |||
parsedKRL = nil | |||
if len(krlFile) > 0 { |
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.
nit, if krlFile != "" {
sorry, i'm an idiot and it's taken me way too long to get to this. |
Hi folks, this is just preliminary to get your thoughts.
Tests and some cleanup are needed. I only did some manual testing on my integration with Tatu (github.com/openstack/tatu).