Skip to content

Hotfix/only admin permission #1844

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

Merged
brylie merged 4 commits into from
Nov 2, 2016
Merged

Hotfix/only admin permission #1844

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c008a68
Replace try/catch bundle to if/else
marla-singer Nov 1, 2016
1ee29c7
Add new helper for checking dashboard page
marla-singer Nov 1, 2016
73381b0
Add condition in navbar for Dashboard button
marla-singer Nov 1, 2016
9483013
Rename helper
marla-singer Nov 2, 2016
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
14 changes: 8 additions & 6 deletions core/client/navbar/navbar.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@
<ul class="nav navbar-nav navbar-left">
{{# if currentUser }}
{{# if proxyIsDefined }}
<li class="{{ isActiveRoute 'dashboard' }}">
<a href="/dashboard">
<i class="fa fa-bar-chart" aria-hidden="true"></i>
{{_ "navbar_dashboard" }}
</a>
</li>
{{# if userCanViewPage }}
Copy link
Copy Markdown
Contributor

@brylie brylie Nov 2, 2016

Choose a reason for hiding this comment

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

This should be called userCanViewDashboard, since we are only checking for permission to view Dashboard here. I.e. 'page' is ambiguous, and is not what we are really checking.

<li class="{{ isActiveRoute 'dashboard' }}">
<a href="/dashboard">
<i class="fa fa-bar-chart" aria-hidden="true"></i>
{{_ "navbar_dashboard" }}
</a>
</li>
{{/ if }}
{{/ if }}
{{/ if }}
<li class="{{ isActiveRoute 'catalogue' }}">
Expand Down
50 changes: 41 additions & 9 deletions core/client/navbar/navbar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,29 +91,61 @@ Template.navbar.helpers({
return false;
},
userCanAddApi () {
try {
// Get settigns document
const settings = Settings.findOne();
// Get settigns document
const settings = Settings.findOne();

if (settings) {
// Get access setting value
const onlyAdminsCanAddApis = settings.access.onlyAdminsCanAddApis;
// If access field doesn't exist, these is false. Allow users to add an API on default
const onlyAdminsCanAddApis = settings.access ? settings.access.onlyAdminsCanAddApis : false;

// Allow user to add an API because not only for admin
if (!onlyAdminsCanAddApis) {
return true;
}

// Otherwise check of user role
// Get current user Id
const userId = Meteor.userId();

// Check if current user is admin
const userIsAdmin = Roles.userIsInRole(userId, ['admin']);

return onlyAdminsCanAddApis && userIsAdmin;
} catch (e) {
// If caught an error, then returning true because no access settings is set
// By default allowing all user to add an API
return true;
return userIsAdmin;
}
// Return true because no settings are set
// By default allowing all user to add an API
return true;
},
userCanViewPage () {
Copy link
Copy Markdown
Contributor

@brylie brylie Nov 2, 2016

Choose a reason for hiding this comment

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

This should be called userCanViewDashboard, since it is only used in that context.

// Allow or not regular user to view Dashboard page
// It depends on onlyAdminsCanAddApis settings

// Get settigns document
const settings = Settings.findOne();

if (settings) {
// Get access setting value
// If access field doesn't exist, these is false. Allow users to view page
const onlyAdminsCanAddApis = settings.access ? settings.access.onlyAdminsCanAddApis : false;

// Allow user to view page because not only for admin
if (!onlyAdminsCanAddApis) {
return true;
}

// Otherwise check of user role
// Get current user Id
const userId = Meteor.userId();

// Check if current user is admin or manager
const userIsAdminOrManager = Roles.userIsInRole(userId, ['admin', 'manager']);

return userIsAdminOrManager;
}
// Return true because no settings are set
// By default allowing all user to add an API
return true;
},
});

Expand Down