Skip to content

Adding Quotes to Sidebar #90

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Adding Quotes to Sidebar #90

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
39cbb5d
Updated Popular Icons to Collapse
devinmatte Oct 28, 2017
daf3626
Updating ignore and Package
devinmatte Oct 28, 2017
1486227
Adding Quotes to sidebar
devinmatte Oct 28, 2017
042f81d
Added Theme Color
devinmatte Nov 4, 2017
9bdb2d1
Fixing URL
devinmatte Nov 13, 2017
6aa64fc
Fixing Newline Issues
devinmatte Feb 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,6 @@ webnews/
webroster/
ybook2003/
yearbook/

# Ignore Config File from git
data/config.json
Copy link
Member

@liam-middlebrook liam-middlebrook Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

3 changes: 3 additions & 0 deletions data/config.tmp.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"quotefaultAPI": "keyherepls"
}
Copy link
Member

@liam-middlebrook liam-middlebrook Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment on ending files with newlines.

10 changes: 8 additions & 2 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<meta name="description" content="">
<meta name="keywords" content="">
<meta name="author" content="">
<meta name="theme-color" content="#B0197E">
<title>CSH Members Portal</title>
<!-- Apple Icon -->
<link rel="apple-touch-icon" sizes="152x152" href="./img/apple-touch-icon.png">
Expand Down Expand Up @@ -47,11 +48,15 @@ <h3 class="panel-title">Popular</h3>
<div class="row">
<div class="col-md-2 col-sm-4 col-xs-6 center" data-ng-repeat="link in popular">
<a data-ng-show="link.href" href="{{ link.href }}" class="icon-box popular-box">
<webicon class= "icon" icon="{{ link.icon }}"></webicon>
<span data-ng-show="showIcons && link.icon">
<webicon class= "icon" icon="{{ link.icon }}"></webicon>
</span>
<p>{{ link.name }}</p>
</a>
<a data-ng-show="link.dropdown" href="#" data-toggle="dropdown" class="icon-box popular-box" id="{{ link.icon }}">
<webicon class="icon" icon="{{ link.icon }}"></webicon>
<span data-ng-show="showIcons && link.icon">
<webicon class="icon" icon="{{ link.icon }}"></webicon>
</span>
<p>{{ link.name }}</p>
</a>
<ul data-ng-show="link.dropdown" class="dropdown-menu" role="menu" aria-labelledby="{{ link.icon }}">
Expand Down Expand Up @@ -116,6 +121,7 @@ <h3 class="panel-title">{{ section.name }}</h3>
<!-- col-md-9 col-sm-12 -->
<!-- Meeting Times -->
<meetings data="meetings"></meetings>
<quote data="quote"></quote>
</div>
</div>
</div>
Expand Down
24 changes: 23 additions & 1 deletion js/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ app.directive("meetings", function() {
};
});

app.directive("quote", function() {
return {
restrict: "E",
templateUrl: "templates/quote.html",
scope: {
quote: "=data"
}
};
});

app.controller("MembersController", ['$scope', '$http', function($scope, $http) {

// Toggle showing the icons
Expand All @@ -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) {
Copy link
Member

@liam-middlebrook liam-middlebrook Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@liam-middlebrook liam-middlebrook Feb 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member Author

@devinmatte devinmatte Feb 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@liam-middlebrook liam-middlebrook Mar 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

$http.get("https://quotefault-api.csh.rit.edu/" + response['quotefaultAPI'] + "/random").success(function (response) {
$scope.quote = response;
}).error(function (error) {
console.error("Error getting quote from API");
});
}).error(function (error) {
console.error("Error getting config.json");
});

// Get all the links
$scope.sections = [];
$scope.popular = [];
$http.get("./data/links.json").success(function (response) {
$scope.sections = response;
// Find the popular links
for (var i = 0; i < $scope.sections.length; i++) {
var section = $scope.sections[i];
var section = $scope.sections[i];
for (var j = 0; j < section.links.length; j++ ) {
if (section.links[j].hasOwnProperty("popular")) {
$scope.popular.push(section.links[j]);
Expand Down
18 changes: 7 additions & 11 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,26 @@
{
"name": "members-redesign",
"name": "CSHMembersPortal",
"description": "Redesign of the CSH Members Portal.",
"version": "0.1.0",
"homepage": "https://github.com/bencentra/members-redesign",
"homepage": "https://github.com/ComputerScienceHouse/CSHMembersPortal",
"author": {
"name": "Ben Centra",
"url": "http://bencentra.com"
},
"repository": {
"type": "git",
"url": "git://github.com/bencentra/members-redesign"
"url": "git://github.com/ComputerScienceHouse/CSHMembersPortal"
},
"bugs": {
"url": "https://github.com/bencentra/members-redesign/issues"
"url": "https://github.com/ComputerScienceHouse/CSHMembersPortal/issues"
},
"licenses": [
{

}
{}
],
"engines": {
"node": ">= 0.10.0"
},
"scripts": {

},
"scripts": {},
"devDependencies": {
"grunt": "~0.4.5",
"grunt-contrib-connect": "0.7.1",
Expand All @@ -34,4 +30,4 @@
"bootstrap",
"csh"
]
}
}
21 changes: 21 additions & 0 deletions templates/quote.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<div class="col-md-3 col-sm-12">
<div class="panel panel-primary center">
<div class="panel-heading">
<div class="panel-title">
<strong>Quotefault!</strong>
</div>
</div>
<div class="panel-body">
<div class="row">
<div class="row">
<div class="col-md-12 col-sm-12 col-xs-12">
<p>"{{ quote.quote }}"</p>
<p>- <b>{{ quote.speaker }}</b></p>
</div>
<hr/>
</div>
</div>
</div>
</div>
</div>

Copy link
Member

@liam-middlebrook liam-middlebrook Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you not have this file end with a double-newline?