Skip to content
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.

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

Add authentication and HTTPS #684

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ coverage
*.tsv
.vscode
.prettierrc

.env
5 changes: 2 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ install: configs
install -d -m 0777 $(DESTDIR)/var/www/images
install -d -m 0777 $(DESTDIR)/var/www/uploads
install -d -m 0777 $(DESTDIR)/var/www/scripts/i18n
install -d -m 0755 $(DESTDIR)/var/www/fonts

cp -a dist/css/*.css $(DESTDIR)/var/www/css
cp -a dist/images/* $(DESTDIR)/var/www/images
Expand All @@ -52,9 +53,7 @@ install: configs
cp -a dist/*.js $(DESTDIR)/var/www/
cp -a dist/*.svg $(DESTDIR)/var/www/
cp -a dist/*.png $(DESTDIR)/var/www/
cp -a dist/*.ttf $(DESTDIR)/var/www/
cp -a dist/*.woff $(DESTDIR)/var/www/
cp -a dist/*.woff2 $(DESTDIR)/var/www/ || :
cp -a dist/fonts/* $(DESTDIR)/var/www/fonts

install -m 0644 dist/404.html $(DESTDIR)/var/www/
install -m 0644 dist/robots.txt $(DESTDIR)/var/www/
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ See into [conventions](https://github.com/wirenboard/conventions/blob/main/READM
## Build & development

Run `npm run build` for building and `npm start` for preview.
To change the mqtt broker url create a "MQTT_BROKER_URI" variable in the .env file with your url.

## Default SVG-Dashboards

Expand Down
35 changes: 25 additions & 10 deletions app/index.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,18 @@
<toast></toast>

<double-bounce-spinner></double-bounce-spinner>

<login-modal show="showLoginModal" http-warning="noHttps"></login-modal>

<div id="wrapper" class="fade" ng-class="{ 'show-console': consoleVisible }">
<exp-check-widget></exp-check-widget>
<div class="alert alert-danger" role="alert" ng-cloak ng-if="noHttps">
<span class="glyphicon glyphicon-exclamation-sign" aria-hidden="true"></span>
<span translate>{{'app.errors.no-https'}}</span>
</div>
<div class="alert alert-danger" role="alert" ng-cloak ng-if="roles.notConfiguredAdmin">
<span class="glyphicon glyphicon-exclamation-sign" aria-hidden="true"></span>
<span translate>{{'app.errors.not-configured-admin'}}</span>
</div>

<!-- Navigation -->
<nav class="navbar navbar-inverse navbar-fixed-top" role="navigation" ng-controller='NavigationCtrl'>
Expand All @@ -38,16 +47,22 @@
<a class="navbar-brand wb-navbar-brand" ui-sref="home">
<img src="images/logo.svg" alt="Wiren Board Web UI">
</a>
<p class="navbar-text navbar-right connection-status">
<span class="label label-custom access-level-label" ng-class="roles.current.roles.isAdmin ? 'label label-custom label-danger' : 'label label-custom label-warning'"
ng-show="roles.current.roles.shortName"
title="{{'navigation.access.title' | translate}} {{roles.current.roles.name | translate}}">
<span class="mobile-screen-access-level">{{roles.current.roles.shortName | translate}}</span>
<span class="big-screen-access-level">{{'navigation.access.title' | translate}} {{roles.current.roles.name | translate}}</span>
</span>
<div class="navbar-text navbar-right connection-status">
<span class="connected-status label label-success" ng-show="isConnected()" translate>{{'navigation.connection.active'}}</span>
<span class="connected-status label label-danger" ng-show="!isConnected()" translate>{{'navigation.connection.inactive'}}</span>
</p>
<div class="dropdown user-menu navbar-right" ng-if="!roles.notConfiguredAdmin">
<i class="glyphicon glyphicon-user" id="userMenu" type="button" data-toggle="dropdown" aria-label="User menu" aria-haspopup="true" aria-expanded="false"></i>
<ul class="dropdown-menu" aria-labelledby="userMenu">
<li>
<span class='user-name' translate>{{roles.current.roles.name}}</span>
</li>
<li role="separator" class="divider"></li>
<li>
<a href="#" ng-click="logout()" translate>{{'app.buttons.logout'}}</a>
</li>
</ul>
</div>
</div>
</div>

<div class="nav navbar-nav navbar-right ma-0"></div>
Expand Down Expand Up @@ -102,7 +117,7 @@
<li>
<a data-toggle="collapse" data-target=".navbar-ex1-collapse" ui-sref="MQTTChannels" ui-sref-active="active">{{'navigation.menu.channels' | translate}}</a>
</li>
<li>
<li ng-if="showAccessControl()">
<a data-toggle="collapse" data-target=".navbar-ex1-collapse" ui-sref="accessLevel" ui-sref-active="active">{{'navigation.menu.access' | translate}}</a>
</li>
<li user-role="mto" current-role="roles.current.role">
Expand Down
107 changes: 82 additions & 25 deletions app/scripts/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@
import AlertCtrl from './controllers/alertController';
import HomeCtrl from './controllers/homeController';
import NavigationCtrl from './controllers/navigationController';
import LoginCtrl from './controllers/loginController';
import MQTTCtrl from './controllers/MQTTChannelsController';
import AccessLevelCtrl from './controllers/accessLevelController';
import DateTimePickerModalCtrl from './controllers/dateTimePickerModalController';
import DiagnosticCtrl from './controllers/diagnosticController';
import BackupCtrl from './controllers/backupController';
Expand Down Expand Up @@ -95,6 +93,8 @@
import confirmDirective from './directives/confirm';
import fullscreenToggleDirective from './directives/fullscreenToggle';
import expCheckMetaDirective from './react-directives/exp-check/exp-check';
import usersPageDirective from './react-directives/users/users';
import loginModalDirective from './react-directives/login/login-modal';
KraPete marked this conversation as resolved.
Show resolved Hide resolved

// Angular routes
import routingModule from './app.routes';
Expand Down Expand Up @@ -176,9 +176,7 @@
.value('AlertDelayMs', 5000)
.controller('AlertCtrl', AlertCtrl)
.controller('HomeCtrl', HomeCtrl)
.controller('LoginCtrl', LoginCtrl)
.controller('MQTTCtrl', MQTTCtrl)
.controller('AccessLevelCtrl', AccessLevelCtrl)
.controller('DateTimePickerModalCtrl', DateTimePickerModalCtrl)
.controller('DiagnosticCtrl', DiagnosticCtrl)
.controller('BackupCtrl', BackupCtrl)
Expand Down Expand Up @@ -266,7 +264,9 @@
.directive('onResize', ['$parse', onResizeDirective])
.directive('ngConfirm', confirmDirective)
.directive('fullscreenToggle', fullscreenToggleDirective)
.directive('expCheckWidget', expCheckMetaDirective);
.directive('expCheckWidget', expCheckMetaDirective)
.directive('usersPage', usersPageDirective)
.directive('loginModal', loginModalDirective);

module
.config([
Expand All @@ -277,7 +277,6 @@
'app',
'console',
'help',
'access',
'mqtt',
'system',
'ui',
Expand Down Expand Up @@ -337,6 +336,56 @@
$rootScope.forceFullscreen = false;
});

function isIp(host) {
return host.match(/^\d+\.\d+\.\d+\.\d+$/);
}

function isLocalDomain(host) {
return host.match(/\.local$/);
}
KraPete marked this conversation as resolved.
Show resolved Hide resolved

async function preStart() {
if (window.location.protocol === 'https:') {
return 'ok';
}

if (isIp(window.location.hostname) || isLocalDomain(window.location.hostname)) {
try {
let response = await fetch('/https/redirect');
if (response.status === 200) {
const https_domain = await response.text();
response = await fetch(`https://${https_domain}/https/check`, {
Copy link
Member

Choose a reason for hiding this comment

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

мне кажется, что тут обязательно проверить, что это тот же контроллер. А то может это другой ответил, или captive portal какой-нибудь, или просто клиент пробросил 80 порт, а на 443 у него какая-нибудь админка роутера.

Я к тому, что этот check должен что-то возвращать, что подтверждает, что:

  1. это WB
  2. этот тот же самый WB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Такое точно возможно?
Если ответит другой, то запрос не пройдёт из-за другого домена в сертификате.

method: 'GET',
mode: 'cors',
});
if (response.status === 200) {
window.location.href = encodeURI(`https://${https_domain}`);

Check warning on line 362 in app/scripts/app.js

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

app/scripts/app.js#L362

Dangerous location.href assignment can lead to XSS. Please use escape(encodeURI(`https://${https_domain}`)) as a wrapper for escaping
KraPete marked this conversation as resolved.
Show resolved Hide resolved
return 'redirected';
}
}
} catch (e) {
/* empty */
}
}
return 'warn';
}

async function getUserType(rolesFactory) {
KraPete marked this conversation as resolved.
Show resolved Hide resolved
try {
let response = await fetch('/auth/who_am_i');
KraPete marked this conversation as resolved.
Show resolved Hide resolved
if (response.status === 200) {
const user = await response.json();
rolesFactory.setRole(user.user_type, false);
return 'ok';
}
if (response.status === 401) {
return 'login';
}
} catch (e) {}

Check warning on line 384 in app/scripts/app.js

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

app/scripts/app.js#L384

Empty block statement.
KraPete marked this conversation as resolved.
Show resolved Hide resolved
rolesFactory.setRole('user', true);
return 'ok';
}

//-----------------------------------------------------------------------------
// Register module with communication
const realApp = angular
Expand Down Expand Up @@ -370,7 +419,8 @@
$translate,
uibDatepickerPopupConfig,
tmhDynamicLocale,
DeviceManagerProxy
DeviceManagerProxy,
rolesFactory
) => {
'ngInject';

Expand Down Expand Up @@ -451,23 +501,14 @@
prefix: $window.localStorage['prefix'],
};

// detect auto url
var autoURL = new URL('/mqtt', $window.location.href);
autoURL.protocol = autoURL.protocol.replace('http', 'ws');
const loginUrl = new URL('/mqtt', $window.location.origin);

// FIXME: I know it's ugly, let's find more elegant way later
var isDev = $window.location.host === 'localhost:8080';
const isDev = $window.location.host === 'localhost:8080';

if (isDev) {
// local debug detected, enable MQTT url override via settings
if (!$window.localStorage.url) {
$window.localStorage.setItem('url', autoURL.href);
}
loginData['url'] = $window.localStorage['url'];
} else {
// no local debug detected, full auto
loginData['url'] = autoURL.href;
if (!isDev) {
loginUrl.protocol = loginUrl.protocol.replace('http', 'ws');
}
loginData.url = loginUrl.href;

let language = localStorage.getItem('language');
const supportedLanguages = ['en', 'ru'];
Expand All @@ -481,8 +522,6 @@
$translate.use(language);
tmhDynamicLocale.set(language);

$rootScope.requestConfig(loginData);

// TBD: the following should be handled by config sync service
var configSaveDebounce = null;
var firstBootstrap = true;
Expand Down Expand Up @@ -528,10 +567,28 @@
}
});

setTimeout(() => {
rolesFactory.whenReady().then(() => {
$rootScope.requestConfig(loginData);
$('double-bounce-spinner').remove();
$('#wrapper').removeClass('fade');
KraPete marked this conversation as resolved.
Show resolved Hide resolved
}, 500);
});

preStart().then(res => {
KraPete marked this conversation as resolved.
Show resolved Hide resolved
if (res === 'redirected') {
return;
}
if (res === 'warn') {
$rootScope.noHttps = true;
}
getUserType(rolesFactory).then(res => {
KraPete marked this conversation as resolved.
Show resolved Hide resolved
if (res === 'login') {
$rootScope.showLoginModal = true;
$rootScope.$apply();
$('double-bounce-spinner').remove();
$('#wrapper').removeClass('fade');
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure '$' Is Defined or Replace With AngularJS Equivalent

The use of $ at lines 587-588 may cause runtime errors if jQuery is not included or $ is not defined. Consistent with AngularJS best practices, please replace $ with angular.element, or ensure that jQuery is properly integrated into the project.

Apply this change:

-               $('double-bounce-spinner').remove();
-               $('#wrapper').removeClass('fade');
+               angular.element('double-bounce-spinner').remove();
+               angular.element('#wrapper').removeClass('fade');

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 eslint

[error] 587-587: '$' is not defined.

(no-undef)


[error] 588-588: '$' is not defined.

(no-undef)

}
});
});
}
);

Expand Down
7 changes: 0 additions & 7 deletions app/scripts/app.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ function routing($stateProvider, $locationProvider, $urlRouterProvider) {
.state('accessLevel', {
url: '/access-level',
template: require('../views/access-level.html'),
controller: 'AccessLevelCtrl as $ctrl',
})
.state('scan', {
url: '/scan',
Expand Down Expand Up @@ -177,12 +176,6 @@ function routing($stateProvider, $locationProvider, $urlRouterProvider) {
},
})
//...........................................................................
.state('login', {
url: '/login/{id}',
template: require('../views/login.html'),
controller: 'LoginCtrl as $ctrl',
})
//...........................................................................
.state('rules', {
url: '/rules',
controller: 'ScriptsCtrl as $ctrl',
Expand Down
17 changes: 0 additions & 17 deletions app/scripts/components/loginForm/loginForm.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,11 @@ class LoginFormCtrl {
constructor($window, $rootScope, $state, $location, rolesFactory) {
'ngInject';

var currentURL = new URL("/mqtt", $window.location.href);
currentURL.protocol = currentURL.protocol.replace('http', 'ws');

this.rootScope = $rootScope;
this.isDev = ($window.location.host === 'localhost:8080'); // FIXME: find more beautiful way to detect local dev
this.localStorage = $window.localStorage;
this.state = $state;
this.rolesFactory = rolesFactory;
this.currentURL = currentURL.href;
this.loginSettings = {};
this.loginSettings.url = this.localStorage['url'];
this.loginSettings.user = this.localStorage['user'];
this.loginSettings.password = this.localStorage['password'];
this.loginSettings.prefix = this.localStorage['prefix'];
Expand All @@ -26,17 +20,11 @@ class LoginFormCtrl {

//...........................................................................
$postLink() {
let url = this.loginSettings.url;
let useCredentials = this.loginSettings.useCredentials;
let user = this.loginSettings.user;
let password = this.loginSettings.password;
let prefix = this.loginSettings.prefix;

if (url) {
this.url = url;
} else {
this.url = this.currentURL;
}
if (useCredentials) {
this.useCredentials = useCredentials;
} else {
Expand All @@ -62,8 +50,6 @@ class LoginFormCtrl {
//...........................................................................
updateLoginSettings() {
// Update settings in Local Storage
if (this.isDev)
this.localStorage.setItem('url', this.url);

this.localStorage.setItem('prefix', this.prefix);

Expand All @@ -77,14 +63,11 @@ class LoginFormCtrl {

// Try to fetch UI config this new settings
let loginData = {
url: this.url,
user: this.user,
password: this.password,
prefix: this.prefix,
isDev: this.isDev,
};

this.rolesFactory.setRole(1);
this.rootScope.requestConfig(loginData);
location.reload();
}
Expand Down
8 changes: 0 additions & 8 deletions app/scripts/components/loginForm/loginForm.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,6 @@ <h3 class="panel-title"><i class="glyphicon glyphicon-log-in"></i> {{'login.titl
<form class="form-horizontal" name="$ctrl.loginForm"
ng-submit="$ctrl.loginForm.$valid ? $ctrl.updateLoginSettings() : ''" novalidate>

<div class="form-group" ng-if="$ctrl.isDev">
<label for="loginURL" class="col-sm-2 control-label" translate>{{'login.labels.url'}}</label>

<div class="col-sm-10">
<input type="text" class="form-control" id="loginURL" name="url" ng-model="$ctrl.url"
required>
</div>
</div>
<div class="form-group">
<div class="col-sm-offset-2 col-sm-10">
<div class="checkbox">
Expand Down
Loading