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

Save the OIDC token on OFN users #5221

Closed
pacodelaluna opened this issue Apr 13, 2020 · 8 comments
Closed

Save the OIDC token on OFN users #5221

pacodelaluna opened this issue Apr 13, 2020 · 8 comments
Assignees

Comments

@pacodelaluna
Copy link

pacodelaluna commented Apr 13, 2020

What is the problem we are solving

We need to save a token from DFC Authorization Server on OFN side in order to link the DFC user with the OFN user, and also deal with authentication.

Success factors = expected outcome

This information is be saved on user side.
In order to be able to reuse this logic for other third-party integrations, let's add an external table to save these Access Tokens.

For the prototype, we can just a this token manually on the enterprise form. Good to block it with Admin control for the prototype.

@luisramos0
Copy link
Contributor

Nice 👍
Here's my feedback:
Are you sure you want to attach a AUTH token to an enterprise? I'd say you want this to be connected to a user, right? And then the user will have permissions to see data from different enterprises in OFN (through enterprise_roles table).
So, I'd put the field on the spree_users table.

Also, I dont think you need to make this DFC specific. It's a generic OIDC that can be used in other interoperability contexts, like single sign on with other app :-)
So, I'd call it oidc_token only.

@pacodelaluna
Copy link
Author

Totally agree to make this more generic, I guess too that it can be reuse for other purposes, oauth is quite common for API auth. But in this case maybe we should use a 1-N relation in order to have the possibility to link more tokens, right? oauth_tokens maybe with fiels like user_id, service_name and token.

About linking it to enterprise object, the idea was to get direct access to the products catalog. We can actually link it to user object too, and then get the enterprise id from the API endpoint. I will update this description about this.

@pacodelaluna
Copy link
Author

pacodelaluna commented Apr 13, 2020

Thinking a bit more, I was also looking at this implementation as an external application making an API call on our system, like if the Authorization initiative was not from our side, and it is the case with DFC prototype requesting for the Catalog. But actually it could be the other way round, we could decide to call the Authorization server to request an an external service. I mean, the general way we will use OAuth will be this way, to call directly an external service, not the contrary.

So maybe it is making more sense to directly use the whole OAuth Consumer stack on our side, through a gem like omniauth-oauth2, https://github.com/omniauth/omniauth-oauth2.

Anyway, in DFC case, we may need this stack to control the Access token on the Authorization server.

@pacodelaluna
Copy link
Author

Well, this approach is not really what we want, we don't want the user to be authenticated through an other auth provider, we just want an user to exchange with a third-party application in a secure way.

It is DFC Prototype who is using an external auth provider for authentication.

Sorry for being confusing. Forget my last comment.

@pacodelaluna pacodelaluna changed the title Save the OIDC token on OFN enterprise Save the OIDC token on OFN users Apr 14, 2020
@luisramos0
Copy link
Contributor

I am not sure how to do this but whatever we do, we should go in the direction of implementing OIDC in OFN as the standard way to authenticate API users.

@luisramos0
Copy link
Contributor

apparently omni auth works with devise (we use devise currently):
https://github.com/heartcombo/devise/wiki/OmniAuth:-Overview

@andrewpbrett
Copy link
Contributor

@pacodelaluna do you agree that #7880 now will close this issue? We have diverged from the original plan but I think that PR solves the essence of this issue.

@RachL
Copy link
Contributor

RachL commented Feb 22, 2022

Closing in favor or : #8922

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants