Skip to content

Commit

Permalink
Allow disabling the skip button (#3077)
Browse files Browse the repository at this point in the history
* Added new disable-skip-button argument

* Added skip button enable backend API

* Added skip button API check in frontend

* Conditionally enable skip button

* Added missing argument to test cases

* Added simple test case for skip button arg

* Declare undefined JS field

* Fix indentation

* Added missing parameter doc

* Added missing API call to test case
  • Loading branch information
onitake authored and k8s-ci-robot committed Jun 11, 2018
1 parent de524c0 commit ded3180
Show file tree
Hide file tree
Showing 14 changed files with 107 additions and 13 deletions.
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

0 comments on commit ded3180

Please sign in to comment.