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

Add Athenz authentication plugin #178

Merged
merged 2 commits into from
Feb 8, 2017
Merged

Add Athenz authentication plugin #178

merged 2 commits into from
Feb 8, 2017

Conversation

nkurihar
Copy link
Contributor

Motivation

Athenz is a role-based authorization system which is recently open-sourced by Yahoo! inc.
We want to use Athenz as an authentication mechanism on pulsar.

Modifications

Add Athenz authentication plugin classes:

  • AuthenticationProviderAthenz (for Broker)
  • AuthenticationAthenz (for Client)
  • AuthenticationDataAthenz (for Client)

and add athenzDomainNames congifuration setting to broker.conf / standalone.conf

Result

Be able to use Athenz authentication on pulsar.

TODO

Add documents about Athenz authentication on pulsar.

@@ -51,5 +51,11 @@
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
</dependency>

<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the Athenz auth provider should be kept in a separate optional module, and not depend on it by default

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' ll move athenz plugin codes to another module directory(like "pulsar-auth-athenz"), thanks.

}

@Override
public String authenticate(AuthenticationDataSource authData) throws AuthenticationException {
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this auth provider interacts with the authorization policies in Pulsar? Do one needs to "grant permission" on the namespace in Pulsar or everything is handled in Athenz?

Copy link
Contributor Author

@nkurihar nkurihar Jan 31, 2017

Choose a reason for hiding this comment

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

everything is handled in Athenz?

No.
In this PR, we use Athenz for only authentication, not authorization.
So users should do "grant permission" on pulsar admin API.

I think to use Athenz for both authentication and authorization is possible,
maybe it needs abstraction of authorization logic on pulsar,
and should be discussed as another issue.

@merlimat merlimat added the type/feature The PR added a new feature or issue requested a new feature label Jan 30, 2017
@merlimat merlimat added this to the 1.17 milestone Jan 30, 2017
Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

Another thing that it would be nice to have is a end-to-end test using Athenz. I'm not sure if it's possible to spawn all the components in the same JVM in the OSS version.

import com.yahoo.pulsar.client.api.PulsarClientException.GettingAuthenticationDataException;

/**
* Created by vmahedia on 10/3/15.
Copy link
Contributor

Choose a reason for hiding this comment

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

Classes authors should be removed

@nkurihar
Copy link
Contributor Author

end-to-end test using Athenz

Do you mean unit tests ?
OK, I think the test is necessary too, I'll try it.

@merlimat
Copy link
Contributor

merlimat commented Feb 6, 2017

@nkurihar The change looks good to me. If it's feasible, it would be good to have a unit test that spawns up an Athenz service and verify the authentication end-to-end.

Also, it would be good to have a new page in the documentation to show how to enable and configure the authentication with Athenz 😄 . But that can go in a different PR.

@nkurihar
Copy link
Contributor Author

nkurihar commented Feb 8, 2017

@merlimat
Thank you for your reviewing.

new page in the documentation to show how to enable and configure the authentication with Athenz

I'll add documents as another PR.

have a unit test that spawns up an Athenz service and verify the authentication end-to-end

I'll investigate and if feasible, add the end-to-end unit test as another PR.

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants