-
Notifications
You must be signed in to change notification settings - Fork 2
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
[PXP-8941] [PXP-8859] Feat/update go 1.17 #25
Conversation
return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"]) | ||
} | ||
func getEmailFromToken(accessTokenVal string) (string, error) { | ||
jwksURL := "http://fence-service/.well-known/openid-configuration" |
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.
should http://fence-service
be a config default instead of hardcoded?
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 does makes sense... But I'm not sure we really need this. Is there a scenario where the fence pod is not running under fence-service
?
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.
if we run the service locally, or in an ecosystem setup like we used to have for NIAID NDE (sub-commons microservices talking to a central fence). But maybe it's too YAGNI 🙂
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.
For running locally, it should be OK (I’m the context of compose-services
, for example).
I’m case of NIAID NDE, I’m not sure on the implications… it’s really depends on who issues the access token for the user and where it runs.
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.
@paulineribeyre I would say — let's merge this for now, and keep an eye on if this will be needed by anyone.
Jira Ticket: PXP-8941
Jira Ticket: PXP-8859
New Features
Breaking Changes
Bug Fixes
x509: certificate signed by unknown authority
Improvements
Dependency updates
Deployment changes
PS: This is follow-up on this one: #24