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.

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

Allow authn and authz methods to be configured #3299

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Allow authn and authz methods to be configured #3299

merged 2 commits into from
Jan 24, 2024

Conversation

simonwo
Copy link
Contributor

@simonwo simonwo commented Jan 23, 2024

This PR exposes configuration options for controlling authn and authz. This actually makes the system usable for controlling access now!

It also includes some of the docs from the docs PR that are relevant for the work at this stage. From this point forward, the docs will track the delivered features.

Resolves https://github.com/bacalhau-project/expanso-planning/issues/341.

@simonwo simonwo requested review from rossjones and frrist January 23, 2024 19:33
Copy link
Contributor

@frrist frrist left a comment

Choose a reason for hiding this comment

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

Good stuff!

Couple nits, otherwise LGTM

cmd/cli/serve/util.go Outdated Show resolved Hide resolved
docs/docs/dev/auth_flow.md Outdated Show resolved Hide resolved
docs/docs/running-node/auth.md Outdated Show resolved Hide resolved
docs/docs/running-node/auth.md Outdated Show resolved Hide resolved
a new **authorization policy**. You can download a policy that restricts
anonymous access and install it by using:

curl -sL https://raw.githubusercontent.com/bacalhau-project/bacalhau/main/pkg/authz/policies/policy_ns_anon.rego -o ~/.bacalhau/no-anon.rego
Copy link
Contributor

@frrist frrist Jan 23, 2024

Choose a reason for hiding this comment

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

we'll need to be careful to never move or renaming policy_ns_anon.rego. Maybe it would be better to reference this via a commit or a version? I don't really have a good suggestion here, just worried that it would be easy for docs to become out of date with this reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or perhaps we could embed this rego policy in the binary? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess with a specific commit we'd then need to remember to update this if we make fixes or improvements. Either way it's just a careful review that is needed. Perhaps we can introduce an automated test or something?

Yeah, I don't actually imagine this being the long term user experience for doing this – but I think it's appropriate right now for beta users. I can imagine in the future we'd have explicit commands to set up users and things, or at least a dedicated tool, but we need to know more about what auth types people commonly want to use.

Is there any value in having something like bacalhau auth install policy_ns_anon or something? Just seems to be a bit overkill for where we're at?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any value in having something like bacalhau auth install policy_ns_anon or something? Just seems to be a bit overkill for where we're at?

seems like a good thing to keep in our back pocket for later, I think you are right - overkill for now.

specifying a new **authentication policy**. You can download a policy that
restricts key-based access and install it by using:

curl -sL https://raw.githubusercontent.com/bacalhau-project/bacalhau/main/pkg/authn/challenge/challenge_ns_no_anon.rego -o ~/.bacalhau/challenge_ns_no_anon.rego
Copy link
Contributor

Choose a reason for hiding this comment

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

similar comment to above.

@simonwo simonwo enabled auto-merge (rebase) January 23, 2024 23:38
@simonwo simonwo merged commit 3031612 into main Jan 24, 2024
11 checks passed
@simonwo simonwo deleted the auth-config branch January 24, 2024 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants