Adds Personal Access Tokens for API access (fixes #541) #1075

andrewn · 2019-05-14T15:22:46Z

Implements Personal Access Tokens for API access. This builds on top of the excellent work in #731 to allow access tokens to be created by users and used to authenticate instead of a password.

I plan on making the following changes from #731:

Move token generation to the server-side from the client. This seems to be how other providers do this (Digital Ocean, Github, Harvest).
Displays tokens as part of Account page (moving from the Advanced page)
Write UserController tests for creating/deleting keys
Improve display of tokens in UI
Write UI tests <- I'll do these in a future PR

I have verified that this pull request:

has no linting errors (npm run lint)
is from a uniquely-named feature branch and has been rebased on top of the latest master. (If I was asked to make more changes, I have made sure to rebase onto master then too)
is descriptively named and links to an issue number, i.e. Fixes #123

UI

I've added tabs to the Account screen. The Access Tokens tab allow tokens to be created and deleted.

Automated testing

I've written tests for the controller endpoints but created Mongoose model mocks by hand. This isn't ideal so I'd like to re-visit these in a future PR once we've figured out a good way forward for testing.

Manual test plan

Create a new access token at /account
Copy access token and use curl to access the test endpoint:
curl -X GET --user <username>:<access-token> -i localhost:8000/api/auth/access-check
If successful, the user's session data should be returned and Last used time in UI should update.
Change username, access should be forbidden
Change token, access should be forbidden
Delete token via UI
API access should be forbidden

The tests mock the mogoose User model and the express Response model which isn't good. We should find a solution that makes use of the actual model object.

server/models/user.js

This ensures it's not accidentally exposed to the client when returning the key metadata

catarak · 2019-05-21T20:49:52Z

client/styles/components/_api-key.scss

+  }
+
+  tbody tr:nth-child(odd) {
+    background-color: #f2f2f2;


could this use a color in the theme map rather than a raw color value?

The account page doesn't respect the theme, but it probably should?

Ok, I've started work on this in #1080.

catarak · 2019-05-21T20:59:28Z

Some other stuff I noticed:

I created a new account, and then immediately opened "Settings -> API Tokens" and the application crashed. It worked totally fine after I refreshed the page.
Extremely small UI fix, could use some space between these two elements (same as the form labels/inputs):

Just... thinking about the overall design of this page. The border for the tabs feels a little funky, though I'm not sure how to change it.

catarak · 2019-05-21T20:59:56Z

but i ran through your test plan and it's working for me!

andrewn · 2019-05-22T13:52:44Z

Just... thinking about the overall design of this page. The border for the tabs feels a little funky, though I'm not sure how to change it.

I know, I'm not very happy with the layout of the page. There's a design for it in #392 but I'm not convinced that it improves things much as none of the fields look editable.

The tabs here don't really work in the full-width page, but we could adopt a side-navigation like GitHub does?

This is until this page's components are made theme-aware

There's duplication in the user and session endpoints that all return the same shaped user model data. The new helper should keep them consistent when new properties need to be exposed.

andrewn · 2019-05-22T16:01:46Z

@catarak, I made those fixes so this is ready for a re-review.

I created a new account, and then immediately opened "Settings -> API Tokens" and the application crashed. It worked totally fine after I refreshed the page.

I fixed this in 5777be3 by ensuring the API consistently returns apiKeys in the user-shaped response.

Extremely small UI fix, could use some space between these two elements (same as the form labels/inputs)

Added some padding around the button.

catarak · 2019-05-22T18:26:09Z

I was just thinking about merging this to master and then this being available in production—maybe it makes sense to merge it to another branch until this is built out more? Maybe it could be visible by a secret url or something? It's hard since this project doesn't have a staging deployment, though it could!

catarak · 2019-05-22T19:00:32Z

one option for the design, that would keep the tabs at the top, would be to wrap the settings container in a box (e.g. like the login design here) so that the tab strip wouldn't be floating.

andrewn · 2019-05-23T09:56:03Z

I was just thinking about merging this to master and then this being available in production—maybe it makes sense to merge it to another branch until this is built out more? Maybe it could be visible by a secret url or something? It's hard since this project doesn't have a staging deployment, though it could!

That's a very good point. I've created a new branch feature/public-api for this purpose and changed the base branch of this PR.

I'll create a new PR for merging feature/public-api into master and use that to track the status of the API work.

It'd be great to have a deployment of this branch for testing. How much effort is that do you think?

Alternatively, we could deploy to production and hide behind feature flags.

catarak · 2019-05-23T20:42:58Z

It'd be great to have a deployment of this branch for testing. How much effort is that do you think?

I think, with the power of CI/CD and Kubernetes, it's not too hard. What I'd need to do:

add staging_deploy script/task to Travis, which will be extremely similar to deploy.sh
update Kubernetes cluster config with new deployment/namespace for staging
add new repository on Dockerhub for staging image
add these variables to Travis

I might wait to start on this until we merge in your .travis.yml changes, so we don't have to deal with conflicts there.

andrewn · 2019-05-27T09:57:33Z

Now that #1081 is merged, this is ready too. 👍

catarak · 2019-05-28T22:08:29Z

Now that #1081 is merged, this is ready too. 👍

great, i can work on setting up the staging deployment this week!

trych · 2020-11-08T10:20:44Z

Whatever happened to the API access features? Is there any plan to implement them?
The public-api branch doesn't seem to exist anymore?

catarak · 2020-11-09T16:47:08Z

@trych the API has been implemented but it hasn't been released yet, but there are plans for this soon. Stay tuned :)

trych · 2020-12-12T12:03:42Z

@catarak

One more question regarding the API:
I want to include some sketches in a blog-like article via the embed feature, so I am dealing with iframes. I am having trouble to automatically get the height of my sketch, as this is of course a cross-origin scenario, so I cannot read the canvas' height via JS.

Is this something that could/would be solved by the API? As in: I would be able to access the sketch height via the API? Or are there even any other solutions for that, that I am missing?

(I could also open a separate issue for that, if that helps)

catarak · 2020-12-14T16:27:08Z

@trych great question! I think this is a separate issue, so I'm going to open one and answer your question there 😄

Vertmo and others added 16 commits May 13, 2019 17:23

Added design of the API key page

e5180bf

Added DB schema and backend logic for API keys creation and deletion

4e23cf7

You can now generate keys from the advanced settings interface

09a8eef

Improved interface for copying key after creation

cfca51d

Hashing keys before storing them

fb18dde

Added Basic Auth using passport-http

88b6e34

Update test snapshots

ee30700

Adds passport-http to package-lock.json

effd299

Moves API key creation to server

74ba9ac

Updates client UI to request token generation from server

64aef54

Removes AdvancedSettingsView as functionality now in AccountView

6425332

Do not return any keys in API

824715c

Fixes API controller tests

23f155c

The tests mock the mogoose User model and the express Response model which isn't good. We should find a solution that makes use of the actual model object.

Displays all API keys in a table, including new token information

30cfca4

Aligns AccountView to top of page

f5d8e6f

Copy button copies token to clipboard

c961c65

andrewn changed the title ~~[WIP] Adds Personal Access Tokens for API access (fixes ##541)~~ [WIP] Adds Personal Access Tokens for API access (fixes #541) May 14, 2019

andrewn commented May 14, 2019

View reviewed changes

server/models/user.js Show resolved Hide resolved

andrewn added 6 commits May 15, 2019 12:11

Styles Account and APIKeys components

29a59b2

lastUserAt should be null if the key has never been used

842e1ba

Removes hashedKey from APIKey data when serialising

9dc0ce4

This ensures it's not accidentally exposed to the client when returning the key metadata

Adds a temporary route for checking Personal Access Tokens work

b77e296

Fixes bug where lastUsedAt timestamp wasn't set when access token used

c60fa70

Fixes linting errors

16363ab

andrewn changed the title ~~[WIP] Adds Personal Access Tokens for API access (fixes #541)~~ Adds Personal Access Tokens for API access (fixes #541) May 15, 2019

andrewn marked this pull request as ready for review May 15, 2019 14:35

andrewn added 2 commits May 15, 2019 16:39

Changes API_KEY_REMOVED action constant to match API_KEY_CREATED

df0099d

Fixes Account back behaviour

66de34d

catarak requested changes May 21, 2019

View reviewed changes

andrewn mentioned this pull request May 22, 2019

Theme aware user pages #1080

Merged

8 tasks

andrewn added 3 commits May 22, 2019 16:30

Adds spacing around social login button

6560a60

Forces light theme on Account page

8c577a4

This is until this page's components are made theme-aware

Consistent user data returned from API

5777be3

There's duplication in the user and session endpoints that all return the same shaped user model data. The new helper should keep them consistent when new properties need to be exposed.

andrewn changed the base branch from master to feature/public-api May 23, 2019 09:48

catarak merged commit 7c4f180 into processing:feature/public-api May 29, 2019

catarak mentioned this pull request May 29, 2019

Add staging configuration and deployment #1086

Closed

4 tasks

andrewn deleted the generate-api-keys branch June 4, 2019 10:37

andrewn mentioned this pull request Jun 6, 2019

API authentication with Token (or oauth?) #541

Open

1 task

catarak mentioned this pull request Dec 14, 2020

Controlling canvas size in embedded iframe #1706

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds Personal Access Tokens for API access (fixes #541) #1075

Adds Personal Access Tokens for API access (fixes #541) #1075

andrewn commented May 14, 2019 •

edited

Loading

catarak May 21, 2019

andrewn May 22, 2019

andrewn May 22, 2019

catarak commented May 21, 2019 •

edited

Loading

catarak commented May 21, 2019

andrewn commented May 22, 2019

andrewn commented May 22, 2019

catarak commented May 22, 2019

catarak commented May 22, 2019

andrewn commented May 23, 2019

catarak commented May 23, 2019

andrewn commented May 27, 2019

catarak commented May 28, 2019

trych commented Nov 8, 2020

catarak commented Nov 9, 2020

trych commented Dec 12, 2020 •

edited

Loading

catarak commented Dec 14, 2020

Adds Personal Access Tokens for API access (fixes #541) #1075

Adds Personal Access Tokens for API access (fixes #541) #1075

Conversation

andrewn commented May 14, 2019 • edited Loading

UI

Automated testing

Manual test plan

catarak May 21, 2019

Choose a reason for hiding this comment

andrewn May 22, 2019

Choose a reason for hiding this comment

andrewn May 22, 2019

Choose a reason for hiding this comment

catarak commented May 21, 2019 • edited Loading

catarak commented May 21, 2019

andrewn commented May 22, 2019

andrewn commented May 22, 2019

catarak commented May 22, 2019

catarak commented May 22, 2019

andrewn commented May 23, 2019

catarak commented May 23, 2019

andrewn commented May 27, 2019

catarak commented May 28, 2019

trych commented Nov 8, 2020

catarak commented Nov 9, 2020

trych commented Dec 12, 2020 • edited Loading

catarak commented Dec 14, 2020

andrewn commented May 14, 2019 •

edited

Loading

catarak commented May 21, 2019 •

edited

Loading

trych commented Dec 12, 2020 •

edited

Loading