Adding Quotes to Sidebar #90
Conversation
| @@ -113,3 +113,6 @@ webnews/ | |||
| webroster/ | |||
| ybook2003/ | |||
| yearbook/ | |||
|
|
|||
| # Ignore Config File from git | |||
| data/config.json | |||
There was a problem hiding this comment.
Could you have the file end with a newline? For reference on why this is important please visit: https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "quotefaultAPI": "keyherepls" | |||
| } | |||
There was a problem hiding this comment.
See above comment on ending files with newlines.
| @@ -32,14 +42,26 @@ app.controller("MembersController", ['$scope', '$http', function($scope, $http) | |||
| console.error("Error getting meetings.json"); | |||
| }); | |||
|
|
|||
| // Get the quotes | |||
| $scope.quote = []; | |||
| $http.get("./data/config.json").success(function (response) { | |||
There was a problem hiding this comment.
Doesn't this pose a security vulnerability where I could visit https://members.csh.rit.edu/data/config.json and then read out your API information and gain access to use that API as your user?
There was a problem hiding this comment.
Thinking back I think this concern could be fairly dismissed since only people who would already have quotefault access would get to this point. The only route that would really lead to any questionable activity would be adding quotes, but if that ends up being a concern a good solution may be to add read-only keys to QuotefaultAPI (although I don't think we'll get to that point).
There was a problem hiding this comment.
I can likely add readonly keys, but yeah in the meantime I'm not really worried about people accessing the key. Also couldn't we do something with permissions around the file to protect it? I don't really know what other way to store the key that makes sense.
There was a problem hiding this comment.
Since you'd be trying to access the file from client-side JS you'd need to make the file readable over HTTP to authenticated clients. I think that designing a way for that file to be secure would require more effort than would be worth it for protecting a database of quotes.
If you end up adding readonly keys I think that would be the simplest way to get a secure solution (from the perspective of someone with organization-level access accessing the API and mutating data through a key that 'anonymizes' them).
I'm fine with this change if @mbillow and @stevenmirabito also agree that this concern isn't high enough priority to block adding in this feature.
templates/quote.html
Outdated
| </div> | ||
| </div> | ||
| </div> | ||
|
|
There was a problem hiding this comment.
Could you not have this file end with a double-newline?
|
@stevenmirabito and I talked about this and the best way to do it is to grab the |
|
@mbillow sounds good. I'll dismiss my pending review for that then and defer to you and @stevenmirabito. |
liam-middlebrook
dismissed
their stale review
Deferring to @mbillow and @stevenmirabito for additional review for JWT integration with Keycloak for security purposes.
|
@devinmatte with the new quotefault API, this will probably have to be refactored. |
Added Quotes to the Sidebar of members using the new Quotefault API!
This pull request includes a few changes:
Fixes #54