Add jwt support for thing-url-adapter #96
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If url -- set auth to bearer with jwt If wss -- Add to query string Load jwt_auth object at start from jwt_auth.json file. In the same dir as addon hostname.local:port : token
move jwt_auth.json to .mozilla-iot/config
… config page. added secureThings property add to db as: <hostname>:<port> <jwt> with a space between
mrstegeman
suggested changes
Sep 28, 2020
…496151126 Renamed getHeaders to getHeaders().
…496152411 Change query param for websocket to ?jwt=<token>
mrstegeman
suggested changes
Sep 29, 2020
MarcGraham
commented
Sep 29, 2020
…497069312 remove unneeded ws==null check
…496936672 Modify manifest.json to collect parameters for jwt, basic and digest auth. Store in config. Update load thing to pull data for auth from config url data. Massaged functions as needed to accomodate the auth stuff. Only JWT is implemented in the adapter code. Stubs are in place to add basic and digest. Will have to add code at lines 52, 297, and 856
mrstegeman
suggested changes
Sep 30, 2020
…497652290 #96 (comment) #96 (comment) Proper naming, remove unneeded comments, remove console.log of AUTH_DATA
…497654154 Add config file update to loadThingURLAdapter.
…497654154 Add 'none' to auth mehtods. On upgrade of config and authenticaton object is added with a method property of 'none'
…497691362 move getHeaders function and authData to ThingURLAdapter. Added adapter property to ThingURLDevice and set it in the constructor. Changed all the getHeader() call accordingly.
…497691362 Update read me with instructions for secure thing usage. Change rev to 5.0 in manifest and package.jsnon
…496149251 Types and grammar.
…496149251 Types and grammar.
mrstegeman
suggested changes
Oct 1, 2020
Remove redundant this.adapter Use part of authentication object for authData Typos and capitolization
mrstegeman
suggested changes
Oct 1, 2020
Contributor
mrstegeman
left a comment
mrstegeman
left a comment
There was a problem hiding this comment.
We're really close! Only very minor things now. Thanks for being patient with me.
thing-url-adapter.js
Outdated
|
|
||
| for (const i in this.adapter.authData) { | ||
| if (this.wsUrl.includes(i)) { | ||
| switch (this.adapter.authData[i].method) { // 0 is the method - jwt etc |
Contributor
There was a problem hiding this comment.
nit: the comment here can be removed now
thing-url-adapter.js
Outdated
| performAction(action) { | ||
| action.start(); | ||
|
|
||
| const headers = this.adapter.getHeaders(this.actionsUrl); |
Contributor
There was a problem hiding this comment.
This one does need the Content-Type header.
thing-url-adapter.js
Outdated
| for (const i in this.authData) { | ||
| if (this.authData.hasOwnProperty(i)) { | ||
| if (url.includes(i)) { | ||
| switch (this.authData[i].method) { // 0 is the method - jwt etc |
thing-url-adapter.js
Outdated
| // add auth to query string | ||
| let auth = ''; | ||
|
|
||
| for (const i in this.adapter.authData) { |
Contributor
There was a problem hiding this comment.
This would be a little cleaner:
for (const [url, authData] of Object.entries(this.adapter.authData))
Contributor
Author
There was a problem hiding this comment.
updated -- this was actually helpful.
thing-url-adapter.js
Outdated
| if (contentType) { | ||
| headers['Content-Type'] = 'application/json'; | ||
| } | ||
| for (const i in this.authData) { |
Contributor
There was a problem hiding this comment.
This would be a little cleaner, and would remove the need for the proceeding hasOwnProperty check:
for (const [url, authData] of Object.entries(this.adapter.authData))
thing-url-adapter.js
Outdated
| for (const url of config.urls) { | ||
| if ('authentication' in url) { | ||
| // remove http(s) from url | ||
| let url_stub = ''; |
Contributor
There was a problem hiding this comment.
nit: urlStub instead of url_stub
Correct getHeaders for eventsUrl and perfromAction Rename variable url_stub
mrstegeman
approved these changes
Oct 2, 2020
Contributor
Author
|
Sweet -- nice to see it in. I noticed something that could be simplified at line 858
Can probably just be adapter.authData[urlStub] = url.authentication; |
Contributor
|
Good call. Done. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Will use token if one available.
Add Authorization header to requests, adds same header to query string -- ?Authorization=Bearer jwt -- for web socket connections.
Tokens can be entered in the thing-url-adapter config page. Use format hostname:port jwt with a space between
I was able to test everything except the performAction and cancelAction functions.