-
Notifications
You must be signed in to change notification settings - Fork 8
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
Moving access related logic from jujuapi to jimm. #1122
Conversation
If you're new to commit signing, there are different ways to set it up: Sign commits with
|
d7a43a9
to
09802de
Compare
internal/jujuapi/controllerroot.go
Outdated
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.
I believe some of the methods on the JIMM interface were due to logic in jujuapi
that should have been in jimm
, like DB()
and AuthorizationClient()
, can those now be removed from the JIMM interface?
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.
in a followup
// (1)[group](2)[-](3)[alices-wonderland](10)[#member] | ||
// So if a group, user, UUID, controller name comes in, it will always be index 3 for them | ||
// and if a relation specifier is present, it will always be index 10 | ||
jujuURIMatcher = regexp.MustCompile(`([a-zA-Z0-9]*)(\-|\z)([a-zA-Z0-9-@.]*)(\:|)([a-zA-Z0-9-@]*)(\/|)([a-zA-Z0-9-]*)(\.|)([a-zA-Z0-9-]*)([a-zA-Z#]*|\z)\z`) |
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 seems this pattern can use some polishing (e.g., removing unnecessary escapes, or simplifying (\:|)
with (:?)
), but considering the goal of the PR, I think it's best to leave it as is.
user := dbmodel.User{ | ||
Username: username, | ||
} | ||
if err := j.Database.GetUser(ctx, &user); err != 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.
In several (if not many) places we do this manually. I mean, we fetch a user from the database and wrap it as a openfga.User
struct instance. Maybe in a follow-up PR, we refactor those cases toward using this method.
…reconfigured realm and client
…reconfigured realm and client
…k/jimm into css-6713/integrate-keycloak
d30ca48
to
7b052c6
Compare
If you're new to commit signing, there are different ways to set it up: Sign commits with
|
7b052c6
to
ca228d7
Compare
Description
Moves access related logic from the jujuapi package to jimm package.
Engineering checklist
Check only items that apply
Test instructions
Notes for code reviewers