Skip to content

Commit

Permalink
Haar 2260 enable authorization code flow (#60)
Browse files Browse the repository at this point in the history
* Add feature flag for authorization_code grant type

* bugfix - error page grant type missing

* extend types for api authorization code types

* surface on the interface

* hide service details under a feature flag

* fix type issue

* add test for mapping authorization information

* Add and update integration tests

* Update README

* Fix merge bugs

* Add cypress env variable
  • Loading branch information
thomasridd authored Feb 7, 2024
1 parent 344dec0 commit cd06424
Show file tree
Hide file tree
Showing 30 changed files with 285 additions and 72 deletions.
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ Or run tests with the cypress UI:

`npm run int-test-ui`

## Environment variables

The following environment variables can be set to run the application:

`ENABLE_AUTHORIZATION_CODE` - set to `true` to enable the authorization code grant type. Default is `false`.

`ENABLE_SERVICE_DETAILS` - set to `true` to enable the service details section. Default is `false`.

## Change log

Expand Down
1 change: 1 addition & 0 deletions feature.env
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ API_CLIENT_SECRET=clientsecret
SYSTEM_CLIENT_ID=clientid
SYSTEM_CLIENT_SECRET=clientsecret
ENVIRONMENT_NAME=dev
ENABLE_AUTHORIZATION_CODE=true
5 changes: 3 additions & 2 deletions integration_tests/e2e/add-base-client.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ context('Add client page', () => {
addBaseClientGrantPage.clientCredentialsRadio().should('have.attr', 'checked')
})

it('Authorization code is disabled (for now)', () => {
addBaseClientGrantPage.authorizationCodeRadio().should('have.attr', 'disabled')
it('Authorization code is not disabled but is unchecked', () => {
addBaseClientGrantPage.authorizationCodeRadio().should('not.have.attr', 'disabled')
addBaseClientGrantPage.authorizationCodeRadio().should('not.have.attr', 'checked')
})

it('User clicks cancel to return to home screen', () => {
Expand Down
5 changes: 3 additions & 2 deletions integration_tests/e2e/edit-base-client-deployment.cy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Page from '../pages/page'
import ViewBaseClientPage from '../pages/viewBaseClient'
import EditBaseClientDeploymentDetailsPage from '../pages/editBaseClientDeploymentDetails'
import { GrantTypes } from '../../server/data/enums/grantTypes'
import AuthSignInPage from '../pages/authSignIn'
import AuthErrorPage from '../pages/authError'

Expand All @@ -15,7 +16,7 @@ context('Edit base client deployment: Auth', () => {
cy.task('stubSignIn')
cy.task('stubManageUser')
cy.task('stubListBaseClients')
cy.task('stubGetBaseClient')
cy.task('stubGetBaseClient', { grantType: GrantTypes.ClientCredentials })
cy.task('stubGetListClientInstancesList')
})

Expand All @@ -40,7 +41,7 @@ context('Edit base client deployment details page', () => {
cy.task('stubSignIn')
cy.task('stubManageUser')
cy.task('stubListBaseClients')
cy.task('stubGetBaseClient')
cy.task('stubGetBaseClient', { grantType: GrantTypes.ClientCredentials })
cy.task('stubGetListClientInstancesList')
editBaseClientDeploymentDetailsPage = visitEditBaseClientDeploymentDetailsPage()
})
Expand Down
49 changes: 42 additions & 7 deletions integration_tests/e2e/edit-base-client-details.cy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Page from '../pages/page'
import EditBaseClientDetailsPage from '../pages/editBaseClientDetails'
import ViewBaseClientPage from '../pages/viewBaseClient'
import { GrantTypes } from '../../server/data/enums/grantTypes'
import AuthSignInPage from '../pages/authSignIn'
import AuthErrorPage from '../pages/authError'

Expand All @@ -15,7 +16,7 @@ context('Edit base client details: Auth', () => {
cy.task('stubSignIn')
cy.task('stubManageUser')
cy.task('stubListBaseClients')
cy.task('stubGetBaseClient')
cy.task('stubGetBaseClient', { grantType: GrantTypes.ClientCredentials })
cy.task('stubGetListClientInstancesList')
})

Expand All @@ -32,15 +33,15 @@ context('Edit base client details: Auth', () => {
})
})

context('Edit base client details page', () => {
context('Edit base client details page - client-credentials flow', () => {
let editBaseClientDetailsPage: EditBaseClientDetailsPage

beforeEach(() => {
cy.task('reset')
cy.task('stubSignIn')
cy.task('stubManageUser')
cy.task('stubListBaseClients')
cy.task('stubGetBaseClient')
cy.task('stubGetBaseClient', { grantType: GrantTypes.ClientCredentials })
cy.task('stubGetListClientInstancesList')
editBaseClientDetailsPage = visitEditBaseClientDetailsPage()
})
Expand All @@ -55,10 +56,18 @@ context('Edit base client details page', () => {
editBaseClientDetailsPage.auditTrailDetailsInput().should('be.visible')
})

it('User can see grant details form inputs', () => {
editBaseClientDetailsPage.grantTypeInput().should('be.visible')
editBaseClientDetailsPage.grantAuthoritiesInput().should('be.visible')
editBaseClientDetailsPage.grantDatabaseUsernameInput().should('be.visible')
context('Grant section for client-credentials flow', () => {
it('User can see client credentials form inputs', () => {
editBaseClientDetailsPage.grantTypeInput().should('be.visible')
editBaseClientDetailsPage.grantAuthoritiesInput().should('be.visible')
editBaseClientDetailsPage.grantDatabaseUsernameInput().should('be.visible')
})

it('User cannot see authorization code form inputs', () => {
editBaseClientDetailsPage.grantRedirectUrisInput().should('not.exist')
editBaseClientDetailsPage.grantJwtFieldsInput().should('not.exist')
editBaseClientDetailsPage.grantAzureAdLoginFlowCheckboxes().should('not.exist')
})
})

it('User can see config form inputs', () => {
Expand Down Expand Up @@ -107,3 +116,29 @@ context('Edit base client details page', () => {
editBaseClientDetailsPage.saveButton().click()
})
})

context('Edit base client details page - authorization-code flow', () => {
let editBaseClientDetailsPage: EditBaseClientDetailsPage

beforeEach(() => {
cy.task('reset')
cy.task('stubSignIn')
cy.task('stubManageUser')
cy.task('stubListBaseClients')
cy.task('stubGetBaseClient', { grantType: GrantTypes.AuthorizationCode })
cy.task('stubGetListClientInstancesList')
editBaseClientDetailsPage = visitEditBaseClientDetailsPage()
})

it('User cannot see client credentials form inputs', () => {
editBaseClientDetailsPage.grantAuthoritiesInput().should('not.exist')
editBaseClientDetailsPage.grantDatabaseUsernameInput().should('not.exist')
})

it('User can see authorization code form inputs', () => {
editBaseClientDetailsPage.grantTypeInput().should('be.visible')
editBaseClientDetailsPage.grantRedirectUrisInput().should('be.visible')
editBaseClientDetailsPage.grantJwtFieldsInput().should('be.visible')
editBaseClientDetailsPage.grantAzureAdLoginFlowCheckboxes().should('exist')
})
})
3 changes: 2 additions & 1 deletion integration_tests/e2e/edit-client-instances.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import Page from '../pages/page'
import ViewBaseClientPage from '../pages/viewBaseClient'
import ViewClientSecretsPage from '../pages/viewClientSecrets'
import ConfirmDeleteClientPage from '../pages/confirmDeleteClient'
import { GrantTypes } from '../../server/data/enums/grantTypes'

const visitBaseClientPage = (): ViewBaseClientPage => {
cy.signIn({ failOnStatusCode: true, redirectPath: '/base-clients/base_client_id_1' })
Expand All @@ -20,7 +21,7 @@ context('Base client page - client instances', () => {
beforeEach(() => {
cy.task('reset')
cy.task('stubSignIn')
cy.task('stubGetBaseClient')
cy.task('stubGetBaseClient', { grantType: GrantTypes.ClientCredentials })
cy.task('stubManageUser')
cy.task('stubGetListClientInstancesList')
})
Expand Down
3 changes: 2 additions & 1 deletion integration_tests/e2e/login.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ import IndexPage from '../pages/index'
import AuthSignInPage from '../pages/authSignIn'
import Page from '../pages/page'
import AuthManageDetailsPage from '../pages/authManageDetails'
import { GrantTypes } from '../../server/data/enums/grantTypes'
import AuthErrorPage from '../pages/authError'

context('SignIn', () => {
beforeEach(() => {
cy.task('reset')
cy.task('stubSignIn', ['ROLE_OAUTH_ADMIN'])
cy.task('stubListBaseClients')
cy.task('stubGetBaseClient')
cy.task('stubGetBaseClient', { grantType: GrantTypes.ClientCredentials })
cy.task('stubManageUser')
})

Expand Down
5 changes: 3 additions & 2 deletions integration_tests/e2e/view-base-client-list.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import Page from '../pages/page'
import ViewBaseClientListPage from '../pages/viewBaseClientList'
import ViewBaseClientPage from '../pages/viewBaseClient'
import NewBaseClientGrantPage from '../pages/newBaseClientGrant'
import { GrantTypes } from '../../server/data/enums/grantTypes'
import AuthSignInPage from '../pages/authSignIn'
import AuthErrorPage from '../pages/authError'

Expand All @@ -16,7 +17,7 @@ context('Homepage - Auth', () => {
cy.task('reset')
cy.task('stubSignIn')
cy.task('stubListBaseClients')
cy.task('stubGetBaseClient')
cy.task('stubGetBaseClient', { grantType: GrantTypes.ClientCredentials })
cy.task('stubManageUser')
cy.task('stubGetListClientInstancesList')
})
Expand All @@ -41,7 +42,7 @@ context('Homepage - list base-clients', () => {
cy.task('reset')
cy.task('stubSignIn')
cy.task('stubListBaseClients')
cy.task('stubGetBaseClient')
cy.task('stubGetBaseClient', { grantType: GrantTypes.ClientCredentials })
cy.task('stubManageUser')
cy.task('stubGetListClientInstancesList')

Expand Down
48 changes: 45 additions & 3 deletions integration_tests/e2e/view-base-client.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import ViewClientSecretsPage from '../pages/viewClientSecrets'
import ConfirmDeleteClientPage from '../pages/confirmDeleteClient'
import EditBaseClientDetailsPage from '../pages/editBaseClientDetails'
import EditBaseClientDeploymentDetailsPage from '../pages/editBaseClientDeploymentDetails'
import { GrantTypes } from '../../server/data/enums/grantTypes'
import AuthSignInPage from '../pages/authSignIn'
import AuthErrorPage from '../pages/authError'

Expand All @@ -16,7 +17,7 @@ context('Base client page - Auth', () => {
beforeEach(() => {
cy.task('reset')
cy.task('stubSignIn')
cy.task('stubGetBaseClient')
cy.task('stubGetBaseClient', { grantType: GrantTypes.ClientCredentials })
cy.task('stubManageUser')
cy.task('stubGetListClientInstancesList')
cy.task('stubAddClientInstance')
Expand All @@ -35,13 +36,13 @@ context('Base client page - Auth', () => {
})
})

context('Base client page', () => {
context('Base client page - client credentials flow', () => {
let baseClientsPage: ViewBaseClientPage

beforeEach(() => {
cy.task('reset')
cy.task('stubSignIn')
cy.task('stubGetBaseClient')
cy.task('stubGetBaseClient', { grantType: GrantTypes.ClientCredentials })
cy.task('stubManageUser')
cy.task('stubGetListClientInstancesList')
cy.task('stubAddClientInstance')
Expand Down Expand Up @@ -78,6 +79,10 @@ context('Base client page', () => {
baseClientsPage.baseClientClientCredentialsTable().should('be.visible')
})

it('User cannot see authorization-code table', () => {
baseClientsPage.baseClientAuthorizationCodeTable().should('not.exist')
})

it('User can see config table', () => {
baseClientsPage.baseClientConfigTable().should('be.visible')
})
Expand All @@ -103,3 +108,40 @@ context('Base client page', () => {
})
})
})

context('Base client page - authorization-code flow', () => {
let baseClientsPage: ViewBaseClientPage

beforeEach(() => {
cy.task('reset')
cy.task('stubSignIn')
cy.task('stubGetBaseClient', { grantType: GrantTypes.AuthorizationCode })
cy.task('stubManageUser')
cy.task('stubGetListClientInstancesList')
cy.task('stubAddClientInstance')

baseClientsPage = visitBaseClientPage()
})

context('Base client details', () => {
it('User can see base client details table', () => {
baseClientsPage.baseClientDetailsTable().should('be.visible')
})

it('User can see audit trail table', () => {
baseClientsPage.baseClientAuditTable().should('be.visible')
})

it('User cannot see client credentials table', () => {
baseClientsPage.baseClientClientCredentialsTable().should('not.exist')
})

it('User can see authorization-code table', () => {
baseClientsPage.baseClientAuthorizationCodeTable().should('be.visible')
})

it('User can see config table', () => {
baseClientsPage.baseClientConfigTable().should('be.visible')
})
})
})
5 changes: 3 additions & 2 deletions integration_tests/mockApis/baseClientsApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getListClientInstancesResponseMock,
getSecretsResponseMock,
} from '../../server/data/localMockData/baseClientsResponseMock'
import { GrantTypes } from '../../server/data/enums/grantTypes'

export default {
stubListBaseClients: () => {
Expand All @@ -23,7 +24,7 @@ export default {
})
},

stubGetBaseClient: () => {
stubGetBaseClient: (config: { grantType: GrantTypes }) => {
return stubFor({
request: {
method: 'GET',
Expand All @@ -34,7 +35,7 @@ export default {
headers: {
'Content-Type': 'application/json;charset=UTF-8',
},
jsonBody: getBaseClientResponseMock,
jsonBody: getBaseClientResponseMock(config.grantType),
},
})
},
Expand Down
2 changes: 2 additions & 0 deletions server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export default {
password: process.env.REDIS_PASSWORD,
tls_enabled: get('REDIS_TLS_ENABLED', 'false'),
},
enableAuthorizationCode: get('ENABLE_AUTHORIZATION_CODE', 'false') === 'true',
enableServiceDetails: get('ENABLE_SERVICE_DETAILS', 'false') === 'true',
session: {
secret: get('SESSION_SECRET', 'app-insecure-default-session', requiredInProduction),
expiryMinutes: Number(get('WEB_SESSION_TIMEOUT_IN_MINUTES', 120)),
Expand Down
4 changes: 2 additions & 2 deletions server/controllers/baseClientController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ describe('BaseClientController', () => {
await baseClientController.displayNewBaseClient()(request, response, next)

// THEN the choose client type page is rendered
expect(response.render).toHaveBeenCalledWith('pages/new-base-client-grant.njk')
expect(response.render).toHaveBeenCalledWith('pages/new-base-client-grant.njk', expect.anything())
})

it('if grant is specified with client-credentials renders the details screen', async () => {
Expand Down Expand Up @@ -218,7 +218,7 @@ describe('BaseClientController', () => {
await baseClientController.displayNewBaseClient()(request, response, next)

// THEN the choose client type page is rendered
expect(response.render).toHaveBeenCalledWith('pages/new-base-client-grant.njk')
expect(response.render).toHaveBeenCalledWith('pages/new-base-client-grant.njk', expect.anything())
})

it('if validation fails because no id specified renders the details screen with error message', async () => {
Expand Down
7 changes: 5 additions & 2 deletions server/controllers/baseClientController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import baseClientAudit, { BaseClientAuditFunction } from '../audit/baseClientAud
import { BaseClientEvent } from '../audit/baseClientEvent'
import { Client } from '../interfaces/baseClientApi/client'
import { mapFilterToUrlQuery, mapListBaseClientRequest } from '../mappers/baseClientApi/listBaseClients'
import config from '../config'

const { enableAuthorizationCode } = config

export default class BaseClientController {
constructor(private readonly baseClientService: BaseClientService) {}
Expand Down Expand Up @@ -69,7 +72,7 @@ export default class BaseClientController {
return async (req, res) => {
const { grant } = req.query
if (!(grant === kebab(GrantTypes.ClientCredentials) || grant === kebab(GrantTypes.AuthorizationCode))) {
res.render('pages/new-base-client-grant.njk')
res.render('pages/new-base-client-grant.njk', { enableAuthorizationCode })
return
}
res.render('pages/new-base-client-details.njk', {
Expand Down Expand Up @@ -283,7 +286,7 @@ export default class BaseClientController {
private renderCreateBaseClientErrorPage(res: Response, error: string, baseClient: BaseClient) {
res.render('pages/new-base-client-details.njk', {
errorMessage: { text: error },
grant: baseClient.grantType,
grant: kebab(baseClient.grantType),
baseClient,
presenter: editBaseClientPresenter(baseClient),
...nunjucksUtils,
Expand Down
6 changes: 6 additions & 0 deletions server/data/enums/mfaTypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// eslint-disable-next-line no-shadow,import/prefer-default-export
export enum MfaType {
None = 'NONE',
All = 'ALL',
Untrusted = 'UNTRUSTED',
}
Loading

0 comments on commit cd06424

Please sign in to comment.