From dc9d7ae3f1011447d345326f413b970cbe5fb09f Mon Sep 17 00:00:00 2001 From: Matt Hook Date: Mon, 8 Apr 2024 11:03:52 +1200 Subject: [PATCH] fix(apikey): don't authenticate api key for external auth [EE-6932] (#11460) --- .../handler/users/user_create_access_token.go | 36 +++++++++++++++--- .../CreateUserAcccessToken.validation.tsx | 17 +++++++-- .../CreateUserAccessToken.test.tsx | 5 ++- .../CreateUserAccessToken.tsx | 18 +++++++-- .../CreateUserAccessTokenInnerForm.tsx | 38 +++++++++++-------- .../DisplayUserAccessToken.tsx | 2 +- .../account/CreateAccessTokenView/types.ts | 2 +- 7 files changed, 88 insertions(+), 30 deletions(-) diff --git a/api/http/handler/users/user_create_access_token.go b/api/http/handler/users/user_create_access_token.go index 373a5ecac..a0be36cd0 100644 --- a/api/http/handler/users/user_create_access_token.go +++ b/api/http/handler/users/user_create_access_token.go @@ -2,6 +2,7 @@ package users import ( "errors" + "fmt" "net/http" portainer "github.com/portainer/portainer/api" @@ -20,9 +21,6 @@ type userAccessTokenCreatePayload struct { } func (payload *userAccessTokenCreatePayload) Validate(r *http.Request) error { - if govalidator.IsNull(payload.Password) { - return errors.New("invalid password: cannot be empty") - } if govalidator.IsNull(payload.Description) { return errors.New("invalid description: cannot be empty") } @@ -44,6 +42,7 @@ type accessTokenResponse struct { // @summary Generate an API key for a user // @description Generates an API key for a user. // @description Only the calling user can generate a token for themselves. +// @description Password is required only for internal authentication. // @description **Access policy**: restricted // @tags users // @security jwt @@ -94,9 +93,21 @@ func (handler *Handler) userCreateAccessToken(w http.ResponseWriter, r *http.Req return httperror.InternalServerError("Unable to find a user with the specified identifier inside the database", err) } - err = handler.CryptoService.CompareHashAndData(user.Password, payload.Password) + internalAuth, err := handler.usesInternalAuthentication(portainer.UserID(userID)) if err != nil { - return httperror.Forbidden("Current password doesn't match", errors.New("Current password does not match the password provided. Please try again")) + return httperror.InternalServerError("Unable to determine the authentication method", err) + } + + if internalAuth { + // Internal auth requires the password field and must not be empty + if govalidator.IsNull(payload.Password) { + return httperror.BadRequest("Invalid request payload", errors.New("invalid password: cannot be empty")) + } + + err = handler.CryptoService.CompareHashAndData(user.Password, payload.Password) + if err != nil { + return httperror.Forbidden("Current password doesn't match", errors.New("Current password does not match the password provided. Please try again")) + } } rawAPIKey, apiKey, err := handler.apiKeyService.GenerateApiKey(*user, payload.Description) @@ -107,3 +118,18 @@ func (handler *Handler) userCreateAccessToken(w http.ResponseWriter, r *http.Req w.WriteHeader(http.StatusCreated) return response.JSON(w, accessTokenResponse{rawAPIKey, *apiKey}) } + +func (handler *Handler) usesInternalAuthentication(userid portainer.UserID) (bool, error) { + // userid 1 is the admin user and always uses internal auth + if userid == 1 { + return true, nil + } + + // otherwise determine the auth method from the settings + settings, err := handler.DataStore.Settings().Settings() + if err != nil { + return false, fmt.Errorf("unable to retrieve the settings from the database: %w", err) + } + + return settings.AuthenticationMethod == portainer.AuthenticationInternal, nil +} diff --git a/app/react/portainer/account/CreateAccessTokenView/CreateUserAcccessToken.validation.tsx b/app/react/portainer/account/CreateAccessTokenView/CreateUserAcccessToken.validation.tsx index 9a4c63073..0a38e0ac3 100644 --- a/app/react/portainer/account/CreateAccessTokenView/CreateUserAcccessToken.validation.tsx +++ b/app/react/portainer/account/CreateAccessTokenView/CreateUserAcccessToken.validation.tsx @@ -2,11 +2,22 @@ import { SchemaOf, object, string } from 'yup'; import { ApiKeyFormValues } from './types'; -export function getAPITokenValidationSchema(): SchemaOf { +export function getAPITokenValidationSchema( + requirePassword: boolean +): SchemaOf { + if (requirePassword) { + return object({ + password: string().required('Password is required.'), + description: string() + .max(128, 'Description must be at most 128 characters') + .required('Description is required.'), + }); + } + return object({ - password: string().required('Password is required.'), + password: string().optional(), description: string() .max(128, 'Description must be at most 128 characters') - .required('Description is required'), + .required('Description is required.'), }); } diff --git a/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessToken.test.tsx b/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessToken.test.tsx index 750e759b2..7c6c66687 100644 --- a/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessToken.test.tsx +++ b/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessToken.test.tsx @@ -33,7 +33,10 @@ test('the button is disabled when all fields are blank and enabled when all fiel }); function renderComponent() { - const user = new UserViewModel({ Username: 'user' }); + const user = new UserViewModel({ + Username: 'admin', + Id: 1, + }); const Wrapped = withTestQueryProvider( withUserProvider(withTestRouter(CreateUserAccessToken), user) diff --git a/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessToken.tsx b/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessToken.tsx index 88eacfbf3..8f5137a5f 100644 --- a/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessToken.tsx +++ b/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessToken.tsx @@ -3,10 +3,13 @@ import { useState } from 'react'; import { useCurrentUser } from '@/react/hooks/useUser'; import { useAnalytics } from '@/react/hooks/useAnalytics'; +import { AuthenticationMethod } from '@/react/portainer/settings/types'; import { Widget } from '@@/Widget'; import { PageHeader } from '@@/PageHeader'; +import { usePublicSettings } from '../../settings/queries/usePublicSettings'; + import { ApiKeyFormValues } from './types'; import { getAPITokenValidationSchema } from './CreateUserAcccessToken.validation'; import { useCreateUserAccessTokenMutation } from './useCreateUserAccessTokenMutation'; @@ -23,6 +26,11 @@ export function CreateUserAccessToken() { const { user } = useCurrentUser(); const [newAPIToken, setNewAPIToken] = useState(''); const { trackEvent } = useAnalytics(); + const settings = usePublicSettings(); + + const requirePassword = + settings.data?.AuthenticationMethod === AuthenticationMethod.Internal || + user.Id === 1; return ( <> @@ -43,12 +51,16 @@ export function CreateUserAccessToken() { - + ) : ( - DisplayUserAccessToken(newAPIToken) + )} diff --git a/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessTokenInnerForm.tsx b/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessTokenInnerForm.tsx index 137189788..74b8fc7ec 100644 --- a/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessTokenInnerForm.tsx +++ b/app/react/portainer/account/CreateAccessTokenView/CreateUserAccessTokenInnerForm.tsx @@ -6,7 +6,11 @@ import { LoadingButton } from '@@/buttons'; import { ApiKeyFormValues } from './types'; -export function CreateUserAccessTokenInnerForm() { +interface Props { + showAuthentication: boolean; +} + +export function CreateUserAccessTokenInnerForm({ showAuthentication }: Props) { const { errors, values, handleSubmit, isValid, dirty } = useFormikContext(); @@ -16,21 +20,23 @@ export function CreateUserAccessTokenInnerForm() { onSubmit={handleSubmit} autoComplete="off" > - - - + {showAuthentication && ( + + + + )} diff --git a/app/react/portainer/account/CreateAccessTokenView/types.ts b/app/react/portainer/account/CreateAccessTokenView/types.ts index ad482dbde..fd663a21c 100644 --- a/app/react/portainer/account/CreateAccessTokenView/types.ts +++ b/app/react/portainer/account/CreateAccessTokenView/types.ts @@ -1,4 +1,4 @@ export interface ApiKeyFormValues { - password: string; + password?: string; description: string; }