Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Introduce Authorizer interface #12053

Merged
merged 9 commits into from
Aug 1, 2023
Merged

Introduce Authorizer interface #12053

merged 9 commits into from
Aug 1, 2023

Conversation

monstermunchkin
Copy link
Contributor

No description provided.

@monstermunchkin monstermunchkin changed the title Introduce Authorization interface Introduce Authorizer interface Jul 20, 2023
@monstermunchkin monstermunchkin force-pushed the misc/refactor-authorization branch from c5fda6d to b54f6b9 Compare July 20, 2023 14:17
@monstermunchkin monstermunchkin force-pushed the misc/refactor-authorization branch 2 times, most recently from 284731b to cc1aa13 Compare July 21, 2023 07:26
@monstermunchkin monstermunchkin marked this pull request as ready for review July 27, 2023 13:35
@monstermunchkin
Copy link
Contributor Author

@tomponline I ran the RBAC tests locally, and they succeed.

@tomponline
Copy link
Member

@tomponline I ran the RBAC tests locally, and they succeed.

Great work!

Do we have tests for the TLS cert authorization method?

@tomponline
Copy link
Member

@tomponline I ran the RBAC tests locally, and they succeed.

Could you create a document for how you set that up, as I'd like to see if we can automate those tests too.

tomponline
tomponline reviewed Jul 27, 2023
View reviewed changes
lxd/daemon.go Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Jul 27, 2023
View reviewed changes
lxd/api_project.go Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Jul 27, 2023
View reviewed changes
lxd/api_project.go Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Jul 27, 2023
View reviewed changes
lxd/api_project.go Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Jul 27, 2023
View reviewed changes
lxd/api_project.go Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Jul 27, 2023
View reviewed changes
lxd/api_project.go Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Jul 27, 2023
View reviewed changes
lxd/auth/rbac/rbac.go Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Jul 27, 2023
View reviewed changes
lxd/auth/rbac/rbac.go Outdated Show resolved Hide resolved
tomponline
tomponline reviewed Jul 27, 2023
View reviewed changes
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks really promising, and I think it will make it much easier to add the rebac support.

Thanks!

@monstermunchkin monstermunchkin force-pushed the misc/refactor-authorization branch from cc1aa13 to ad52e7d Compare July 28, 2023 08:40
@monstermunchkin
Copy link
Contributor Author

Do we have tests for the TLS cert authorization method?

We do have tests in tls_restrictions.sh.

@monstermunchkin
Copy link
Contributor Author

Could you create a document for how you set that up, as I'd like to see if we can automate those tests too.

Yes, of course, I'll do that.

@tomponline
Copy link
Member

Do we have tests for the TLS cert authorization method?

We do have tests in tls_restrictions.sh.

Cool, and does this new interface also handle that sort of authorization?

@monstermunchkin
Copy link
Contributor Author

Cool, and does this new interface also handle that sort of authorization?

Yes, that is handled by the CommonAuthorizer.

@monstermunchkin monstermunchkin force-pushed the misc/refactor-authorization branch from ad52e7d to 9cfa4aa Compare July 31, 2023 14:52
@tomponline
Copy link
Member

Hi @monstermunchkin please can you rebase to fix conflict

@monstermunchkin
Copy link
Contributor Author

Hi @monstermunchkin please can you rebase to fix conflict

I'll do that.

I think I'll also remove the Authorizer interface from the state. Since RBAC relies on the state, I don't want to run into any circular dependency issues when adding OpenFGA. Also, removing it from state will make the implementation somewhat cleaner as we can have the files on the same levels, i.e.

  • auth/driver_tls.go
  • auth/driver_rbac.go
  • auth/authorization.go

@monstermunchkin monstermunchkin force-pushed the misc/refactor-authorization branch from 9cfa4aa to f634240 Compare August 1, 2023 10:35
@monstermunchkin
auth: Add common authorization driver
2d09161 
Signed-off-by: Thomas Hipp <[email protected]>
@monstermunchkin
auth: Add tls authorizer
8e8bd69 
Signed-off-by: Thomas Hipp <[email protected]>
@monstermunchkin
lxd: Add Authorizer to daemon
51eeb0e 
Signed-off-by: Thomas Hipp <[email protected]>
@monstermunchkin
state: Add Authorizer to state
2720fbc 
Signed-off-by: Thomas Hipp <[email protected]>
@monstermunchkin
rbac: Make rbac use the new Authorizer interface
3ef847d 
Signed-off-by: Thomas Hipp <[email protected]>
@monstermunchkin
Use Authorizer interface across the code base
85b03d5 
Signed-off-by: Thomas Hipp <[email protected]>
@monstermunchkin
lxd: Stop status check if RBAC is disabled
ea20800 
This ensures the status check (a goroutine) is stopped when RBAC is
disabled.

Signed-off-by: Thomas Hipp <[email protected]>
@monstermunchkin monstermunchkin force-pushed the misc/refactor-authorization branch from 1e83dad to ea20800 Compare August 1, 2023 13:12
@tomponline tomponline merged commit b1b45da into canonical:main Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomponline tomponline Awaiting requested review from tomponline

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@monstermunchkin @tomponline