WebAuth Proof of Concept #1058

Benjamin-Philip · 2021-10-07T12:53:10Z

The following is a Proof of Concept for web auth.

Closes #1024

Benjamin-Philip · 2021-10-07T12:57:33Z

@wojtekmach here's the pr.

lib/hexpm_web/controllers/web_auth_controller.ex

lib/hexpm/web_auth.ex

Benjamin-Philip · 2021-10-07T16:21:02Z

Does everything else look fine?

Benjamin-Philip · 2021-10-14T12:19:35Z

@ericmj , I have got an incomplete implementation in Schema. Whenever I run Repo.update in submit, I get the following error. I figured it might be due to the way I am migrating this, but I am unable to troubleshoot. Any ideas?

** (DBConnection.EncodeError) Postgrex expected an integer in -9223372036854775808..9223372036854775807, got %{id: 2344, inserted_at: ~U[2021-10-14 12:19:23.249932Z], last_use: nil, name: "Web Auth foo key", organization_id: nil, permissions: [%{domain: "api", id: "ad886288-2d8f-4c9c-b888-859b1d4c8953", resource: "write"}], public: true, revoke_at: nil, revoked_at: nil, secret_first: "3c663e623cc7cd32516dfc99c2b3c990", secret_second: "65a86ffcfe4f6299f910e48117cbca71", updated_at: ~U[2021-10-14 12:19:23.249932Z], user_id: 8435}. Please make sure the value you are passing matches the definition in your table or in your query or convert the value accordingly.

ericmj · 2021-10-14T12:37:36Z

I have got an incomplete implementation in Schema. Whenever I run Repo.update in submit, I get the following error. I figured it might be due to the way I am migrating this, but I am unable to troubleshoot. Any ideas?

It's hard to say without seeing what's being updated and when I try to run your branch it fails with a compilation error, but it looks like postgres is expecting an id/integer where you are passing a Key schema or Key params.

I do notice that you are creating a Key with Keys.create and then embedding it in WebAuth which means it is stored in two places which we should try to avoid. Why does WebAuth need to store a Key?

Benjamin-Philip · 2021-10-14T13:34:42Z

@ericmj, I need to return the key when the user attempts to access it in the last step.

Since, the web auth requests are temporary, I cannot do a one to one association.

I could return a url to access the key which I think right now is the best approach , but I am not sure if I can access a generated key via the api.

ericmj · 2021-10-14T13:37:01Z

I need to return the key when the user attempts to access it.

Will the user need to fetch the key multiple times? Isn't it enough to create the key first when the user needs it and then not store it separately?

Benjamin-Philip · 2021-10-14T13:44:23Z

@ericmj Well, the cli won't.

On second thoughts, storing the generated key's primary id and accessing it when the user needs is doable.

Wdyt?

ericmj · 2021-10-14T16:13:11Z

On second thoughts, storing the generated key's primary id and accessing it when the user needs is doable.

I think we should only generate the key when the user needs it and never let them access it again. We should not store the user secret of the key, if you look at the Key code you can see that we only store the hashed version of the key secret. The reason being if the database is compromised you cannot use that information to authenticate to Hex. If the user can retrieve the key multiple times the point of this is moot.

Benjamin-Philip · 2021-10-15T06:14:06Z

If the user can retrieve the key multiple times the point of this is moot.

So, in the GenServer implementation, the request is deleted after few minutes, after which the user can't access the key.

Anyways, we'll generate the key when the user attempts to access it like you said.

Benjamin-Philip · 2021-10-16T13:38:22Z

@ericmj, I have pushed a complete implementation of WebAuth. Please review when you get the time. :)

Note: It doesn't compile on this branch as I have made some changes to the Phoenix Router which I have not committed which it depends on (To determine the verification and access_key uri).

Benjamin-Philip · 2021-10-21T11:29:15Z

@ericmj, also I don't plan to work on the controller so as to avoid doing redundant work in case the Schema implementation is not to your satisfaction.

lib/hexpm/accounts/web_auth.ex

ericmj · 2021-11-16T20:18:20Z

@Benjamin-Philip I noticed you pushed more commits. Let us know when it's ready for second review.

Benjamin-Philip · 2021-11-17T02:08:39Z

Sure.

I only fixed some minor stuff and still have to split the Web auth module into a context. I also have the remove the extra information.

The thing is this and next month, I may not have much time to work on this PR. So updates are going to be slow.

Benjamin-Philip · 2021-11-18T12:00:06Z

Sure.

I only fixed some minor stuff and still have to split the Web auth module into a context. I also have the remove the extra information.

The thing is this and next month, I may not have much time to work on this PR. So updates are going to be slow.

@ericmj, I hope there isn't a hurry?

wojtekmach · 2021-11-18T12:23:49Z

There is no rush :)

Benjamin-Philip · 2021-11-19T09:35:23Z

@ericmj and @wojtekmach, I have split WebAuth into a context and a schema.

I'm not very sure if what I have done is the right way to go about it. (All the functions in the Context are impure, so I think it is.)

Benjamin-Philip · 2021-11-23T06:38:08Z

@ericmj, and @wojtekmach, please review the WebAuthRequest schema and the WebAuth context so that I can make improvements (or plan accordingly to work on them).

Note: Changes to HexpmWeb is not yet review ready.

lib/hexpm/accounts/web_auth.ex

test/hexpm/accounts/web_auth_test.exs

lib/hexpm/accounts/web_auth.ex

lib/hexpm/accounts/web_auth_request.ex

lib/hexpm/accounts/web_auth.ex

Co-authored-by: Eric Meadows-Jönsson <eric.meadows.jonsson@gmail.com>

Previously, the browser user agent was used to creating an audit. This had the following problem: - It gave a false sense on whose request the key was generated. - The user agent was stored under the field `audit` which caused confusion. This commit changes the WebAuth flow so that the client's user agent (that is on the `access_key` action) is used to create a key.

WebAuth requests made a day (i.e. stale) before should not be supported. This commit adds support for the `submit` and `access_key` actions to filter out stale requests when querying the DB.

This commit adds a release task that will remove all stale requests from the database.

This commit adds the RFC that was used as a reference when developing the WebAuth flow as a comment

The audit data format changed and required an ip address. This commit updates the WebAuth PR to use this new format.

Benjamin-Philip · 2022-05-04T12:23:04Z

@ericmj and @josevalim, could we request someone else to review this PR as well?

This PR went out of date and had errors due to changes in main. I think we should try to merge this PR as soon as possible to prevent such errors - updating this PR to work with main periodically may prove a hassle to me and the reviewers.

Benjamin-Philip · 2022-05-04T12:26:24Z

Also I rebased this PR to main.

Benjamin-Philip marked this pull request as ready for review October 7, 2021 12:53

Benjamin-Philip changed the title ~~Bp web auth po c~~ Bp web auth poc Oct 7, 2021

Benjamin-Philip changed the title ~~Bp web auth poc~~ WebAuth Proof of concept Oct 7, 2021

Benjamin-Philip changed the title ~~WebAuth Proof of concept~~ WebAuth Proof of Concept Oct 7, 2021

Benjamin-Philip marked this pull request as draft October 7, 2021 13:48

Benjamin-Philip mentioned this pull request Oct 7, 2021

Web auth to authenticate CLI from browser #1024

Open

wojtekmach reviewed Oct 7, 2021

View reviewed changes

lib/hexpm_web/controllers/web_auth_controller.ex Outdated Show resolved Hide resolved

Benjamin-Philip commented Oct 7, 2021

View reviewed changes

lib/hexpm/web_auth.ex Outdated Show resolved Hide resolved

ericmj reviewed Oct 21, 2021

View reviewed changes

lib/hexpm/accounts/web_auth.ex Show resolved Hide resolved

lib/hexpm/accounts/web_auth.ex Outdated Show resolved Hide resolved

lib/hexpm/accounts/web_auth.ex Outdated Show resolved Hide resolved

lib/hexpm/accounts/web_auth.ex Outdated Show resolved Hide resolved

Benjamin-Philip requested review from wojtekmach and ericmj November 19, 2021 09:32

wojtekmach reviewed Nov 23, 2021

View reviewed changes

lib/hexpm/accounts/web_auth.ex Outdated Show resolved Hide resolved

Benjamin-Philip and others added 24 commits May 4, 2022 16:05

Add success page for successful verification

bbf16c9

Fix html indentation in form

fe56f6d

Co-authored-by: Eric Meadows-Jönsson <eric.meadows.jonsson@gmail.com>

Fix success message indentation

a85ed09

Co-authored-by: Eric Meadows-Jönsson <eric.meadows.jonsson@gmail.com>

Remove redundant JSON reply from submit action

adb1a88

Return key secret in access_key action

26fbe5e

Rename WebAuthBrowser to WebAuth

6ba1fa5

Move API.WebAuth Endpoint from /login to /api

c80913e

Support erlang responses

a4f9100

Support erlang errors

87d2285

Add submit Endpoint for hex tests

012b2b9

Use different names for read and write keys

1a37be7

Remove autocomplete from form

9b0fba8

Make :user_code and :device_code unique independently

e8a302d

Rename :requests table to :web_auth_requests

670300d

Beautify code

6100dfc

Add suggestions for API.WebAuthController

afb21a6

Add suggestion for WebAuthController

6e27260

Add support for stale requests

33ce145

WebAuth requests made a day (i.e. stale) before should not be supported. This commit adds support for the `submit` and `access_key` actions to filter out stale requests when querying the DB.

Add Remove Stale WebAuth Requests tasks

e12b953

This commit adds a release task that will remove all stale requests from the database.

Wrap successful WebAuth.access_key requests in a tuple

998f3ec

Add WebAuth flow reference RFC as comment

050226b

This commit adds the RFC that was used as a reference when developing the WebAuth flow as a comment

Force rerun of CI

2e812f0

Fix errors on test

c3c1eee

The audit data format changed and required an ip address. This commit updates the WebAuth PR to use this new format.

Benjamin-Philip force-pushed the bp-web-auth-PoC branch from 24f988f to c3c1eee Compare May 4, 2022 12:19

Benjamin-Philip closed this Oct 3, 2022

Benjamin-Philip reopened this Oct 3, 2022

ericmj force-pushed the main branch from eba0fa2 to aad402b Compare January 29, 2023 23:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WebAuth Proof of Concept #1058

WebAuth Proof of Concept #1058

Benjamin-Philip commented Oct 7, 2021 •

edited

Benjamin-Philip commented Oct 7, 2021

Benjamin-Philip commented Oct 7, 2021

Benjamin-Philip commented Oct 14, 2021 •

edited

ericmj commented Oct 14, 2021

Benjamin-Philip commented Oct 14, 2021 •

edited

ericmj commented Oct 14, 2021

Benjamin-Philip commented Oct 14, 2021 •

edited

ericmj commented Oct 14, 2021

Benjamin-Philip commented Oct 15, 2021

Benjamin-Philip commented Oct 16, 2021 •

edited

Benjamin-Philip commented Oct 21, 2021

ericmj commented Nov 16, 2021

Benjamin-Philip commented Nov 17, 2021 •

edited

Benjamin-Philip commented Nov 18, 2021

wojtekmach commented Nov 18, 2021

Benjamin-Philip commented Nov 19, 2021

Benjamin-Philip commented Nov 23, 2021

Benjamin-Philip commented May 4, 2022 •

edited

Benjamin-Philip commented May 4, 2022

WebAuth Proof of Concept #1058

Are you sure you want to change the base?

WebAuth Proof of Concept #1058

Conversation

Benjamin-Philip commented Oct 7, 2021 • edited

Benjamin-Philip commented Oct 7, 2021

Benjamin-Philip commented Oct 7, 2021

Benjamin-Philip commented Oct 14, 2021 • edited

ericmj commented Oct 14, 2021

Benjamin-Philip commented Oct 14, 2021 • edited

ericmj commented Oct 14, 2021

Benjamin-Philip commented Oct 14, 2021 • edited

ericmj commented Oct 14, 2021

Benjamin-Philip commented Oct 15, 2021

Benjamin-Philip commented Oct 16, 2021 • edited

Benjamin-Philip commented Oct 21, 2021

ericmj commented Nov 16, 2021

Benjamin-Philip commented Nov 17, 2021 • edited

Benjamin-Philip commented Nov 18, 2021

wojtekmach commented Nov 18, 2021

Benjamin-Philip commented Nov 19, 2021

Benjamin-Philip commented Nov 23, 2021

Benjamin-Philip commented May 4, 2022 • edited

Benjamin-Philip commented May 4, 2022

Benjamin-Philip commented Oct 7, 2021 •

edited

Benjamin-Philip commented Oct 14, 2021 •

edited

Benjamin-Philip commented Oct 14, 2021 •

edited

Benjamin-Philip commented Oct 14, 2021 •

edited

Benjamin-Philip commented Oct 16, 2021 •

edited

Benjamin-Philip commented Nov 17, 2021 •

edited

Benjamin-Philip commented May 4, 2022 •

edited