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

Allow disabling the skip button #3077

Merged
merged 10 commits into from
Jun 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions src/app/backend/args/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@ func (self *holderBuilder) SetDisableSettingsAuthorizer(enableSettingsAuthorizer
return self
}

// SetDisableSkipButton 'disable-skip' argument of Dashboard binary.
func (self *holderBuilder) SetDisableSkipButton(disableSkipButton bool) *holderBuilder {
self.holder.disableSkipButton = disableSkipButton
return self
}

// GetHolderBuilder returns singletone instance of argument holder builder.
func GetHolderBuilder() *holderBuilder {
return builder
Expand Down
7 changes: 7 additions & 0 deletions src/app/backend/args/holder.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ type holder struct {
autoGenerateCertificates bool
enableInsecureLogin bool
disableSettingsAuthorizer bool

disableSkipButton bool
}

// GetInsecurePort 'insecure-port' argument of Dashboard binary.
Expand Down Expand Up @@ -146,3 +148,8 @@ func (self *holder) GetEnableInsecureLogin() bool {
func (self *holder) GetDisableSettingsAuthorizer() bool {
return self.disableSettingsAuthorizer
}

// GetDisableSettingsAuthorizer 'disable-settings-authorizer' argument of Dashboard binary.
func (self *holder) GetDisableSkipButton() bool {
return self.disableSkipButton
}
8 changes: 8 additions & 0 deletions src/app/backend/auth/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ type AuthManager interface {
Refresh(string) (string, error)
// AuthenticationModes returns array of auth modes supported by dashboard.
AuthenticationModes() []AuthenticationMode
// AuthenticationSkippable tells if the Skip button should be enabled or not
AuthenticationSkippable() bool
}

// TokenManager is responsible for generating and decrypting tokens used for authorization. Authorization is handled
Expand Down Expand Up @@ -136,3 +138,9 @@ type TokenRefreshSpec struct {
type LoginModesResponse struct {
Modes []AuthenticationMode `json:"modes"`
}

// LoginSkippableResponse contains a flag that tells the UI not to display the Skip button.
// Note that this only hides the button, it doesn't disable unauthenticated access.
type LoginSkippableResponse struct {
Skippable bool `json:"skippable"`
}
8 changes: 8 additions & 0 deletions src/app/backend/auth/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ func (self AuthHandler) Install(ws *restful.WebService) {
ws.GET("/login/modes").
To(self.handleLoginModes).
Writes(authApi.LoginModesResponse{}))
ws.Route(
ws.GET("/login/skippable").
To(self.handleLoginSkippable).
Writes(authApi.LoginSkippableResponse{}))
}

func (self AuthHandler) handleLogin(request *restful.Request, response *restful.Response) {
Expand Down Expand Up @@ -97,6 +101,10 @@ func (self *AuthHandler) handleLoginModes(request *restful.Request, response *re
response.WriteHeaderAndEntity(http.StatusOK, authApi.LoginModesResponse{Modes: self.manager.AuthenticationModes()})
}

func (self *AuthHandler) handleLoginSkippable(request *restful.Request, response *restful.Response) {
response.WriteHeaderAndEntity(http.StatusOK, authApi.LoginSkippableResponse{Skippable: self.manager.AuthenticationSkippable()})
}

// NewAuthHandler created AuthHandler instance.
func NewAuthHandler(manager authApi.AuthManager) AuthHandler {
return AuthHandler{manager: manager}
Expand Down
20 changes: 13 additions & 7 deletions src/app/backend/auth/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ import (

// Implements AuthManager interface
type authManager struct {
tokenManager authApi.TokenManager
clientManager clientapi.ClientManager
authenticationModes authApi.AuthenticationModes
tokenManager authApi.TokenManager
clientManager clientapi.ClientManager
authenticationModes authApi.AuthenticationModes
authenticationSkippable bool
}

// Login implements auth manager. See AuthManager interface for more information.
Expand Down Expand Up @@ -65,6 +66,10 @@ func (self authManager) AuthenticationModes() []authApi.AuthenticationMode {
return self.authenticationModes.Array()
}

func (self authManager) AuthenticationSkippable() bool {
return self.authenticationSkippable
}

// Returns authenticator based on provided LoginSpec.
func (self authManager) getAuthenticator(spec *authApi.LoginSpec) (authApi.Authenticator, error) {
if len(self.authenticationModes) == 0 {
Expand All @@ -91,10 +96,11 @@ func (self authManager) healthCheck(authInfo api.AuthInfo) error {

// NewAuthManager creates auth manager.
func NewAuthManager(clientManager clientapi.ClientManager, tokenManager authApi.TokenManager,
authenticationModes authApi.AuthenticationModes) authApi.AuthManager {
authenticationModes authApi.AuthenticationModes, authenticationSkippable bool) authApi.AuthManager {
return &authManager{
tokenManager: tokenManager,
clientManager: clientManager,
authenticationModes: authenticationModes,
tokenManager: tokenManager,
clientManager: clientManager,
authenticationModes: authenticationModes,
authenticationSkippable: authenticationSkippable,
}
}
18 changes: 16 additions & 2 deletions src/app/backend/auth/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func TestAuthManager_Login(t *testing.T) {
}

for _, c := range cases {
authManager := NewAuthManager(c.cManager, c.tManager, authApi.AuthenticationModes{authApi.Token: true})
authManager := NewAuthManager(c.cManager, c.tManager, authApi.AuthenticationModes{authApi.Token: true}, true)
response, err := authManager.Login(c.spec)

if !areErrorsEqual(err, c.expectedErr) {
Expand All @@ -166,11 +166,25 @@ func TestAuthManager_AuthenticationModes(t *testing.T) {
}

for _, c := range cases {
authManager := NewAuthManager(cManager, tManager, c.modes)
authManager := NewAuthManager(cManager, tManager, c.modes, true)
got := authManager.AuthenticationModes()

if !reflect.DeepEqual(got, c.expected) {
t.Errorf("Expected %v, but got %v.", c.expected, got)
}
}
}

func TestAuthManager_AuthenticationSkippable(t *testing.T) {
cManager := &fakeClientManager{}
tManager := &fakeTokenManager{}
cModes := authApi.AuthenticationModes{}

for _, flag := range []bool{true,false} {
authManager := NewAuthManager(cManager, tManager, cModes, flag)
got := authManager.AuthenticationSkippable()
if (got != flag) {
t.Errorf("Expected %v, but got %v.", flag, got)
}
}
}
7 changes: 6 additions & 1 deletion src/app/backend/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ var (
argMetricClientCheckPeriod = pflag.Int("metric-client-check-period", 30, "Time in seconds that defines how often configured metric client health check should be run. Default: 30 seconds.")
argAutoGenerateCertificates = pflag.Bool("auto-generate-certificates", false, "When set to true, Dashboard will automatically generate certificates used to serve HTTPS. Default: false.")
argEnableInsecureLogin = pflag.Bool("enable-insecure-login", false, "When enabled, Dashboard login view will also be shown when Dashboard is not served over HTTPS. Default: false.")
argDisableSkip = pflag.Bool("disable-skip", false, "When enabled, the skip button on the login page will not be shown. Default: false.")
argSystemBanner = pflag.String("system-banner", "", "When non-empty displays message to Dashboard users. Accepts simple HTML tags. Default: ''.")
argSystemBannerSeverity = pflag.String("system-banner-severity", "INFO", "Severity of system banner. Should be one of 'INFO|WARNING|ERROR'. Default: 'INFO'.")
argDisableSettingsAuthorizer = pflag.Bool("disable-settings-authorizer", false, "When enabled, Dashboard settings page will not require user to be logged in and authorized to access settings page.")
Expand Down Expand Up @@ -194,7 +195,10 @@ func initAuthManager(clientManager clientapi.ClientManager) authApi.AuthManager
authModes.Add(authApi.Token)
}

return auth.NewAuthManager(clientManager, tokenManager, authModes)
// UI logic dictates this should be the inverse of the cli option
authenticationSkippable := !args.Holder.GetDisableSkipButton()

return auth.NewAuthManager(clientManager, tokenManager, authModes, authenticationSkippable)
}

func initArgHolder() {
Expand All @@ -217,6 +221,7 @@ func initArgHolder() {
builder.SetAutoGenerateCertificates(*argAutoGenerateCertificates)
builder.SetEnableInsecureLogin(*argEnableInsecureLogin)
builder.SetDisableSettingsAuthorizer(*argDisableSettingsAuthorizer)
builder.SetDisableSkipButton(*argDisableSkip)
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/app/backend/handler/apihandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func getTokenManager() authApi.TokenManager {

func TestCreateHTTPAPIHandler(t *testing.T) {
cManager := client.NewClientManager("", "http://localhost:8080")
authManager := auth.NewAuthManager(cManager, getTokenManager(), authApi.AuthenticationModes{})
authManager := auth.NewAuthManager(cManager, getTokenManager(), authApi.AuthenticationModes{}, true)
sManager := settings.NewSettingsManager(cManager)
sbManager := systembanner.NewSystemBannerManager("Hello world!", "INFO")
_, err := CreateHTTPAPIHandler(nil, cManager, authManager, sManager, sbManager)
Expand Down
7 changes: 7 additions & 0 deletions src/app/externs/backendapi.js
Original file line number Diff line number Diff line change
Expand Up @@ -1545,6 +1545,13 @@ backendApi.LoginModesResponse;
*/
backendApi.SupportedAuthenticationModes;

/**
* @typedef {{
* skippable: boolean,
* }}
*/
backendApi.LoginSkippableResponse;

/**
* @typedef {{
* running: number,
Expand Down
19 changes: 18 additions & 1 deletion src/app/frontend/login/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,12 @@ class LoginController {
* @param {!ui.router.$state} $state
* @param {!angular.$q.Promise} kdAuthenticationModesResource
* @param {!../common/errorhandling/service.ErrorService} kdErrorService
* @param {!angular.$q.Promise} kdSkipButtonEnabled
* @ngInject
*/
constructor(kdNavService, kdAuthService, $state, kdAuthenticationModesResource, kdErrorService) {
constructor(
kdNavService, kdAuthService, $state, kdAuthenticationModesResource, kdErrorService,
kdSkipButtonEnabled) {
/** @private {!./../chrome/nav/nav_service.NavService} */
this.kdNavService_ = kdNavService;
/** @private {!./../common/auth/service.AuthService} */
Expand All @@ -62,6 +65,10 @@ class LoginController {
this.supportedAuthenticationModes = Modes;
/** @private {!../common/errorhandling/service.ErrorService} */
this.errorService_ = kdErrorService;
/** @private {!angular.$q.Promise} */
this.skipButtonEnabledResource_ = kdSkipButtonEnabled;
/** @private {boolean} */
this.skipButtonEnabled_;
}

/** @export */
Expand All @@ -81,6 +88,9 @@ class LoginController {
this.authenticationModesResource_.then((authModes) => {
this.enabledAuthenticationModes_ = authModes.modes;
});
this.skipButtonEnabledResource_.then((skippable) => {
this.skipButtonEnabled_ = skippable.skippable;
});
}

/**
Expand All @@ -100,6 +110,13 @@ class LoginController {
return enabled;
}

/**
* @export
*/
isSkipButtonEnabled() {
return this.skipButtonEnabled_;
}

/**
* @param {!backendApi.LoginSpec} loginSpec
* @export
Expand Down
3 changes: 2 additions & 1 deletion src/app/frontend/login/login.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@
[[Sign in|Text shown on the sign in button on login page]]
</md-button>
<md-button class="md-primary"
ng-click="$ctrl.skip()">
ng-click="$ctrl.skip()"
ng-if="$ctrl.isSkipButtonEnabled()">
[[Skip|Text shown on skip button on login page]]
</md-button>
</kd-content>
Expand Down
2 changes: 2 additions & 0 deletions src/app/frontend/login/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {kubeConfigLoginComponent} from './kubeconfig_component';
import {loginOptionsComponent} from './options_component';
import stateConfig from './stateconfig';
import {authenticationModesResource} from './stateconfig';
import {skippableResource} from './stateconfig';
import {tokenLoginComponent} from './token_component';

/**
Expand All @@ -42,4 +43,5 @@ export default angular
.component('kdTokenLogin', tokenLoginComponent)
.component('kdKubeConfigLogin', kubeConfigLoginComponent)
.factory('kdAuthenticationModesResource', authenticationModesResource)
.factory('kdSkipButtonEnabled', skippableResource)
.config(stateConfig);
9 changes: 9 additions & 0 deletions src/app/frontend/login/stateconfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,12 @@ const config = {
export function authenticationModesResource($resource) {
return $resource('api/v1/login/modes').get().$promise;
}

/**
* @param {!angular.$resource} $resource
* @return {!angular.$q.Promise}
* @ngInject
*/
export function skippableResource($resource) {
return $resource('api/v1/login/skippable').get().$promise;
}
4 changes: 4 additions & 0 deletions src/test/frontend/login/component_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ describe('Login component', () => {
let authModesResource;
/** @type {!angular.$q.Deferred} */
let deferred;
/** @type {!angular.$q.Promise} */
let skipButtonEnabledResource;

beforeEach(() => {
angular.mock.module(errorModule.name);
Expand All @@ -43,13 +45,15 @@ describe('Login component', () => {
q = $q;
deferred = q.defer();
authModesResource = deferred.promise;
skipButtonEnabledResource = deferred.promise;
state = $state;
authService = kdAuthService;
scope = $rootScope;
ctrl = $componentController('kdLogin', {
$state: $state,
kdAuthService: authService,
kdAuthenticationModesResource: authModesResource,
kdSkipButtonEnabled: skipButtonEnabledResource,
});

});
Expand Down