dexidp / dex

OpenID Connect (OIDC) identity and OAuth 2.0 provider with pluggable connectors
https://dexidp.io
Apache License 2.0
9.48k stars 1.7k forks source link

grpc connector #1020

Open rio opened 7 years ago

rio commented 7 years ago

Have there been any plans/thoughts on implementing a grpc connector? One that will just implement the PasswordConnector and RefreshConnector interfaces. Just to make it easier for people to use Dex as a frontend for token generation and all that jazz more in line with what Hydra is doing and without needing to fork the project to have your custom connector implemented.

fforootd commented 7 years ago

Hi @Rio

As of today, we decided to do exactly that in an forked version of dex. Our idea was to test it first internally and contribute it afterwards.

I will ask the guy working on the connector tomorrow about the status.

rio commented 7 years ago

That's great! I was just implementing it just to see how it would work. Hopefully we could pull you fork back in.

ericchiang commented 7 years ago

I think a pluggable connector for out-of-tree development would be great. Committing to an interface is probably the biggest design hurdle here.

ericchiang commented 7 years ago

Out of curiosity, what provider are you trying to hook into? My general experience is that making things pluggable is good but also doesn't magically produce more development. It'd be great to understand what the current pain points are.

fforootd commented 7 years ago

We internally use a grpc services as part of our identity solution, somewhat like an ldap but it does only store identity's (username, password and user related data).
The reason to not use an LDAP based approach is that we use the same authentication interface for the auth process between our microservices (every microservice gets itself a token from this service)

So we thought about building a more generic grpc connector where one can define his endpoint and the rpc request name from protobuf, as well as the message parameters to map his own values against the connector's.

rio commented 7 years ago

We are in the process to move stuff over to a new architecture but we of course need to be backwards compatible until it's time. One of the things we're running into is not being able to extend the openid tokens with groups and what not in Dex's local password database using the grpc api.

At the same time we were thinking to not store the users in there at all and since we're going to use grpc more and more we thought to try to see if we could connect Dex to our own user storage until we flesh out if and how we want Dex to do password storage for us and if we can live without the groups extendability for example.

We also have a legacy where the combination of username and password will determine which customer this user will be logged into. So same username but different passwords will get you different customers. Transitioning to a single canonical user record will take time as we use those different user ids for permissions. Which is why having extra logic during the grpc login call to our backend would still be necessary.

Or we'll just have to bite the bullet and merge all users first and update our entire permissions system. What kind of permissions/authz systems are you guys using in combination with Dex?

ericchiang commented 7 years ago

For some background, dex is committed to not becoming an user management or authorization solution. That's why so many of the "local" user objects are so underdeveloped. If we could remove them we would, but they're too important for bootstrapping.

I like the idea of letting external systems manage users without having to implement a crazy protocol like LDAP. I wonder if an establish REST based specification like SCIM might be more appropriate than trying to define our own gRPC API.

http://www.simplecloud.info/

rio commented 7 years ago

Thanks for that link, nice to see some example for user management standardization. However I think that implementing this api will push dex to a more user management function which I feel complicates things. As I noted before, it only needs to be the Login rpc call to be implemented and optionally also Refresh.

Here is an example simple protobuf spec based on the structs and interfaces in https://github.com/coreos/dex/blob/master/connector/connector.go

syntax = "proto3";

package grpc;

message Scopes {
    bool offline_access = 1;
    bool groups = 2;
}

message Identity {
    string user_id = 1;
    string username = 2;
    string email = 3;
    bool email_verified = 4;
    repeated string groups = 5;
}

message LoginRequest {
    string username = 1;
    string password = 2;
    Scopes scopes = 3;
}

message LoginResponse {
    Identity identity = 1;
    bool valid_password = 2;
}

message RefreshRequest {
    Scopes scopes = 1;
    Identity identity = 2;
}

service Connector {
    rpc Login(LoginRequest) returns (LoginResponse) {};
    rpc Refresh(RefreshRequest) returns (Identity) {};
}

This is all we would need to get going.

Edit: Added refresh rpc for completeness.

rio commented 7 years ago

I've commited this and generated code with a version that can complie but not run here btw https://github.com/Rio/dex/tree/1020-grpc-connector

Not sure how far @fforootd is yet but I thought it couldn't hurt.

livio-a commented 7 years ago

Hi @Rio

We came up with more or less the same solution. As we don't need groups at the time of login (they will be loaded later from an other grpc service), I didn't handle them for now. For a general connector I would certainly add them as well. Also the scopes and field email_verified could further be added.

So here's our solution: https://github.com/workshop21/dex/tree/1020-grpc-connector Any suggestions on improvements or what ever would be great...

btw: You asked about the the authz systems we're using. We created as small service (also written in go) where we store permissions (a combination of tenant, user and application) and provide a grpc interface. This interface provides the necessary information for our internal token service which produces a thing that we call "RoleToken". This is basically a JWT with private claims corresponding to the users permissions. We do this out of the reason because we don't want to provide "id_token" who are getting to big to the users. It also solves a problem with token blacklisting, because these "RoleTokens" tend to have a really short lifetime (about 60s).

rio commented 7 years ago

Whoah great! Looks like it should work nicely. It is at the same level feature wise as the local connector if i'm not mistaken so that would be a good start. I do have some questions though, mostly related to my rookieness and OCD I guess ;)

Why would you not just implement a grpc service inside of the protobuf file and use the generated client for it instead of manually setting the grpc methods in the config and calling grpc.Invoke? I can imagine giving somebody the option to use a different method path because of their own implementation if they don't want to or can use a generated server from the protobuf file. But I feel that by default being able to generate a server from the protobuf is a great time saver.

If the plan is to merge this into upstream I feel we should add scopes, email_verified and groups support just to be complete as you suggested. And of course add tests ;) Having a generated client and server for testing could be handy.

Are you running this in production now? Just curious. Also about authz; I get not wanting to throw around big id_tokens. What kind of authz style are you using RBAC? ABAC? Something else? And what libraries? Sorry for the loads of questions, I'm still figuring out what I want to use for permissions and looking at others is the best thing imho.

rio commented 7 years ago

I also see that google/api/annotations.proto is imported in grpc.proto but not used. You wanted to use grpc-gateway?

ericchiang commented 7 years ago

Quick point: we haven't committed to a gRPC interface so it's way too early to be digging into the implementation.

As we've hit with Kubernetes, making an internal interface external through a webhook or gRPC means that it's almost impossible to change that interface in the future. For example, what if we wanted to add a new, required field to the login response? Today we could make that change all in one place. With this proposal we break compatibility with our external system.

That's why I pointed to solutions like SCIM. At least someone else has done the hard work of establishing the protocol contracts.

I personally don't want to commit to the existing connector interfaces until the end of time. If the proposal here is "create a gRPC interface that mirrors the current Go interfaces" I'm against it.

fforootd commented 7 years ago

Quick note:

We use SCIM 2.0 as well but it does only define the identity exchange. It does not address things like authentification. On Fri, 11 Aug 2017 at 17:49, Eric Chiang notifications@github.com wrote:

Quick point: we haven't committed to a gRPC interface so it's way too early to be digging into the implementation.

As we've hit with Kubernetes, making an internal interface external through a webhook or gRPC means that it's almost impossible to change that interface in the future. For example, what if we wanted to add a new, required field to the login response? Today we could make that change all in one place. With this proposal we break compatibility with our external system.

That's why I pointed to solutions like SCIM. At least someone else has done the hard work of establishing the protocol contracts.

I personally don't want to commit to the existing connector interfaces until the end of time. If the proposal here is "create a gRPC interface that mirrors the current Go interfaces" I'm against it.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/coreos/dex/issues/1020#issuecomment-321849533, or mute the thread https://github.com/notifications/unsubscribe-auth/AJbBqD_RyAegL-6lTGs-RPd8SJgT0YVhks5sXHglgaJpZM4OyNu1 .

fforootd commented 7 years ago

@ericchiang i will quickly clarify what we are trying to achieve.

The gRPC Interface should be just an other connector similar to an ldap connector, which connects to our IDP. Our IDP stores all the necessary information about the user identity and also provides a SCIM interface for identity provisioning with third parties, but this has nothing todo with DEX.

I unterstand your point about not want to commit to the existing Go Interface until end of the world :-) But in that case, if something changes, i guess it would be sufficient to refactor the gRPC connector like all the others, or do i get something wrong here?

ericchiang commented 7 years ago

The gRPC Interface should be just an other connector similar to an ldap connector, which connects to our IDP. Our IDP stores all the necessary information about the user identity and also provides a SCIM interface for identity provisioning with third parties, but this has nothing todo with DEX.

Yep. I'm trying to identify if there's an actual real protocol we can implement that would allow dex to connect to your IdP, such as SCIM.

I unterstand your point about not want to commit to the existing Go Interface until end of the world :-) But in that case, if something changes, i guess it would be sufficient to refactor the gRPC connector like all the others, or do i get something wrong here?

We can't control the server side implementation of that gRPC interface (the thing the connector would connect to). Dex would only be able to modify that interface in ways that maintained backwards compatibility with the external interface.

This would prevent us form doing things like refactoring the connector to understand per-user data instead of just per-login data (https://github.com/coreos/dex/issues/863#issuecomment-294997858).

glerchundi commented 6 years ago

@ericchiang @rithujohn191

I know that it's preferable to let other specifications do the hard job but I think including this extension point would open up dex to wide variety of integrations.

I tried to draw the current possibilities to attach dex to a custom user database and I found myself connecting a myriad of services down the road (dex->LDAP->{inmem,sql or dex->SCIM->custom connector or ...). It's crazy. This proposal would be a real, easy and practically immediate way to get things done.

So the question is, is there any chance with this issue or should we start thinking in other solutions?

vyshane commented 6 years ago

I have also been looking at integrating dex with an external user service, and have arrived at a similar solution: A connector that implements the PasswordConnector and RefreshConnector and talks to the external service via gRPC. Then I found this issue.

What can we do to get something like this merged into dex? Would love to not have to maintain a fork of dex just so I can use this connector.

srenatus commented 6 years ago

I'd like to revive this. Having a generic gRPC connector seems very worthwhile.

@ericchiang @rithujohn191 What do you think, is the concern from above (from https://github.com/dexidp/dex/issues/1020#issuecomment-322518187) still valid, and blocking this?

I would suppose that a future direction could also include some sort of backwards-compatibility (with potential limitations), but in the near term, I think a gPRC connector could unlock many simplifications for adopters. Also, it could be a counter-proposal to all the "enhance user-management" requests. 🤔

Update: Thinking about it, any future changes on the gRPC interface would perhaps be simpler to adjust with for people than what happens right now: complete forks (👋 https://github.com/concourse/dex), or embedding for custom connectors (#1288).

ericchiang commented 6 years ago

If it reduces the number of people asking for user management in dex, I'd be open to exposing a gRPC interface for sending a username and password.

I will note that that's what SCIM is though 😃 (REST instead of gRPC):

http://www.simplecloud.info/ https://tools.ietf.org/html/rfc7644#section-3.2 https://tools.ietf.org/html/rfc7644#section-3.4.1 http://www.simplecloud.info/#Implementations2

amdonov commented 6 years ago

I'd like to see the interface support more data than is available in the current Identity object. Assuming dex is going to to support a UserInfo endpoint eventually, it would be nice to have a flexible way to return attributes that are connector specific. For example, birthday, picture, etc.

ericchiang commented 6 years ago

If someone wants to send a SCIM or gRPC PR I'm happy to review it.

nabokihms commented 2 years ago

Related issue https://github.com/dexidp/dex/issues/1907