-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
Major improvements to auth #983
Conversation
c8cf295
to
369e9f6
Compare
|
369e9f6
to
cfffd00
Compare
cfffd00
to
5f3c529
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.
LGTM, just minor comments
return fmt.Errorf("table identifiers has an unsupported count: %d", len(auth.TargetNames)) | ||
} | ||
for i := 0; i < len(auth.TargetNames); i += 3 { | ||
// TODO: handle database |
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.
Might do a check for current DB and error if it doesn't match
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.
Same comment elsewhere this happens
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 think we access other DBs in the workbench. At least, tests seemingly related to those fail when I enforce this. So I reverted it and left these as TODO
s for now
5f3c529
to
50f35fe
Compare
50f35fe
to
ce8fa1e
Compare
ce8fa1e
to
af650d2
Compare
This adds a myriad of tests, in addition to:
There are also many smaller changes, such as disallowing non-superusers from modifying superusers, and many more.