datopian / portal.js.bak

🌀 The JS data presentation framework. For a single dataset to a full catalog.
https://portal-78qurbwf9-datopian1.vercel.app/
MIT License
22 stars 2 forks source link

Feature/authentication #53

Closed steveoni closed 3 years ago

steveoni commented 3 years ago

This pull request adds an authentication feature to Portal.js. This authentication feature makes use of Auth0.

For subsequent users, all that needs to be done is to create an Auth0 account and obtain the following keys, and then add them to the environmental variables

domain=
clientId=
clientSecret=
redirectUri=http://localhost:3000/api/callback
postLogoutRedirectUri=http://localhost:3000/
cookieSecret=any-long-gibrish-text
vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/datopian1/portal/50ci98oji
✅ Preview: Failed

anuveyatsu commented 3 years ago

@steveoni before I start reviewing, I'd like to understand where is the issue for this PR? I couldn't find one in the list of issues.

Note that adding auth system is a significant change and we should make sure we're using right solution here. We've done some analysis of different options earlier this year and Auth0 was on of the options, however, we have selected https://github.com/ory/kratos. Although, Auth0 could be the right option for Portal.JS, we need to make sure that we have done analysis.

Also, this PR has merge conflicts.

steveoni commented 3 years ago

@steveoni before I start reviewing, I'd like to understand where is the issue for this PR? I couldn't find one in the list of issues.

Note that adding auth system is a significant change and we should make sure we're using right solution here. We've done some analysis of different options earlier this year and Auth0 was on of the options, however, we have selected https://github.com/ory/kratos. Although, Auth0 could be the right option for Portal.JS, we need to make sure that we have done analysis.

Also, this PR has merge conflicts.

The issue for this PR is in #34 .

The following are the reason I went for AuthO:

anuveyatsu commented 3 years ago

@steveoni thanks for the answer.

I think the main point of using Auth0 is that there are examples of how to use it with NextJS. All other points are also available in other solutions. I'm fine with Auth0 in general but I wanted to check with @rufuspollock if this is something we want to go with (hosted auth vs something we host ourselves).

rufuspollock commented 3 years ago

@anuveyatsu i would agree with your views here. I think we need some more thought/analysis in #34 before we commit to a solution. Let's do more on #34 first. For now i think we can just leave this and come back to it when we need auth.

@steveoni want to acknowledge that it is great you jumped in and we have something and i am sure we can progress again once we are clearer what we need 😄

rufuspollock commented 3 years ago

WONTMERGE. This is now out of date and would relate to examples/catalog. If we want to re-use later we can do that but for now this is out of date so closing.