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

Improve our approach for testing auth (part 1) #9681

Merged
merged 29 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
764582e
improve stackexchange auth testing
jNullj Oct 23, 2023
2e913a7
Merge branch 'master' into feat/9493/improve-auth-testing
jNullj Oct 23, 2023
ae1a231
Merge branch 'badges:master' into feat/9493/improve-auth-testing
jNullj Jan 6, 2024
b2c2a18
Merge branch 'badges:master' into feat/9493/improve-auth-testing
jNullj Jan 6, 2024
9dd597d
Remove dummy auth test
jNullj Jan 16, 2024
7bc3cc0
Add getBadgeExampleCall to test-helpers
jNullj Jan 20, 2024
f6da3af
Use getBadgeExampleCall in stackexchange-base tests
jNullj Jan 20, 2024
31c3f94
Fix getBadgeExampleCall Errors
jNullj Jan 20, 2024
18ec387
Add testAuth to test-helpers
jNullj Jan 20, 2024
1688e58
Refactor stackexchange-base.spec.js to use testAuth from test-helpers
jNullj Jan 20, 2024
609c017
Split stackexchange-base.spec into per service test file
jNullj Jan 20, 2024
ffc7800
Add all auth methods to testAuth
jNullj Feb 10, 2024
3e5c98d
Handle non-default bearer and api headers
jNullj Feb 11, 2024
876708f
Add discord.spec.js as first attempt for bearer auth
jNullj Feb 11, 2024
1ddd577
Merge branch 'badges:master' into feat/9493/improve-auth-testing
jNullj Feb 11, 2024
c41f60f
Fix basic auth user
jNullj Feb 11, 2024
f4cc1af
Add dynamic authorizedOrigins
jNullj Feb 11, 2024
b471c5c
Add header optional argument
jNullj Feb 11, 2024
7aadc10
Add obs as basicAuth example
jNullj Feb 11, 2024
79dc536
Use apiHeaderKey and bearerHeaderKey function params
jNullj Feb 13, 2024
d1435c2
Remove old comment
jNullj Feb 13, 2024
a53f716
Allow any pass & user key for QueryStringAuth
jNullj Feb 16, 2024
14d0789
Add auth test for PepyDownloads
jNullj Feb 16, 2024
d22de8a
Fix wrong header for jwt login
jNullj Feb 16, 2024
50f4144
Support multiple authOrigins in testAuth
jNullj Feb 16, 2024
2d310bd
Add docker-automated auth test
jNullj Feb 16, 2024
1b79b4c
Fix JwtAuth testing by introducing mandatory jwtLoginEndpoint
jNullj Feb 17, 2024
419bd01
Merge branch 'badges:master' into feat/9493/improve-auth-testing
jNullj Feb 17, 2024
a2b838c
Fix type test in generateFakeConfig
jNullj Feb 20, 2024
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
38 changes: 0 additions & 38 deletions services/stackexchange/stackexchange-base.spec.js

This file was deleted.

10 changes: 10 additions & 0 deletions services/stackexchange/stackexchange-monthlyquestions.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { testAuth } from '../test-helpers.js'
import StackExchangeMonthlyQuestions from './stackexchange-monthlyquestions.service.js'

describe('StackExchangeMonthlyQuestions', function () {
describe('auth', function () {
it('sends the auth information as configured', async function () {
return testAuth(StackExchangeMonthlyQuestions, { total: 8 })
})
})
})
10 changes: 10 additions & 0 deletions services/stackexchange/stackexchange-reputation.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { testAuth } from '../test-helpers.js'
import StackExchangeReputation from './stackexchange-reputation.service.js'

describe('StackExchangeReputation', function () {
describe('auth', function () {
it('sends the auth information as configured', async function () {
return testAuth(StackExchangeReputation, { items: [{ reputation: 8 }] })
})
})
})
10 changes: 10 additions & 0 deletions services/stackexchange/stackexchange-taginfo.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { testAuth } from '../test-helpers.js'
import StackExchangeQuestions from './stackexchange-taginfo.service.js'

describe('StackExchangeQuestions', function () {
describe('auth', function () {
it('sends the auth information as configured', async function () {
return testAuth(StackExchangeQuestions, { items: [{ count: 8 }] })
})
})
})
97 changes: 96 additions & 1 deletion services/test-helpers.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { expect } from 'chai'
import nock from 'nock'
import config from 'config'
import { fetch } from '../core/base-service/got.js'
import BaseService from '../core/base-service/base.js'
const runnerConfig = config.util.toObject()

function cleanUpNockAfterEach() {
Expand Down Expand Up @@ -30,6 +32,99 @@ function noToken(serviceClass) {
}
}

/**
* Retrieves an example set of parameters for invoking a service class using OpenAPI example of that class.
*
* @param {BaseService} serviceClass The service class containing OpenAPI specifications.
* @returns {object} An object with call params to use with a service invoke of the first OpenAPI example.
* @throws {TypeError} - Throws a TypeError if the input `serviceClass` is not an instance of BaseService,
* or if it lacks the expected structure.
*
* @example
* // Example usage:
* const example = getBadgeExampleCall(StackExchangeReputation)
* console.log(example)
* // Output: { stackexchangesite: 'stackoverflow', query: '123' }
* StackExchangeReputation.invoke(defaultContext, config, example)
*/
function getBadgeExampleCall(serviceClass) {
if (!(serviceClass.prototype instanceof BaseService)) {
throw new TypeError(
'Invalid serviceClass: Must be an instance of BaseService.',
)
}

if (!serviceClass.openApi) {
throw new TypeError(
`Missing OpenAPI in service class ${serviceClass.name}.`,
)
}

const firstOpenapiPath = Object.keys(serviceClass.openApi)[0]

const firstOpenapiExampleParams =
serviceClass.openApi[firstOpenapiPath].get.parameters
if (!Array.isArray(firstOpenapiExampleParams)) {
throw new TypeError(
`Missing or invalid OpenAPI examples in ${serviceClass.name}.`,
)
}

// reformat structure for serviceClass.invoke
const exampleInvokeParams = firstOpenapiExampleParams.reduce((acc, obj) => {
acc[obj.name] = obj.example
return acc
}, {})

return exampleInvokeParams
}

/**
* Test authentication of a badge for it's first OpenAPI example using a provided dummyResponse
*
* @param {BaseService} serviceClass The service class tested.
* @param {object} dummyResponse An object containing the dummy response by the server.
* @throws {TypeError} - Throws a TypeError if the input `serviceClass` is not an instance of BaseService,
* or if `serviceClass` is missing authorizedOrigins.
*
* @example
* // Example usage:
* testAuth(StackExchangeReputation, { items: [{ reputation: 8 }] })
*/
async function testAuth(serviceClass, dummyResponse) {
if (!(serviceClass.prototype instanceof BaseService)) {
throw new TypeError(
'Invalid serviceClass: Must be an instance of BaseService.',
)
}

cleanUpNockAfterEach()

const config = { private: { stackapps_api_key: 'fake-key' } }
Copy link
Member

Choose a reason for hiding this comment

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

This is specific to the stackexchange badges. If this is going to be a completely generic helper we can use for all services, we'll need to pass this into testAuth as a param. I'd suggest we make the config object the first or second param to testAuth(). This one is small, so lets address this one before merging.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So i went a bit overboard and just made everything generic and support all auth methods.
So now i have a function to generate a fake config object based on the service class.
See ffc7800

const exampleInvokeParams = getBadgeExampleCall(serviceClass)
const authOrigin = serviceClass.auth.authorizedOrigins[0]

if (!authOrigin) {
throw new TypeError(`Missing authorizedOrigins for ${serviceClass.name}.`)
}

const scope = nock(authOrigin)
.get(/.*/)
.query(queryObject => queryObject.key === 'fake-key')
Copy link
Member

Choose a reason for hiding this comment

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

Again, this is a bit specific to this service. This will work for services that pass auth in the query string, but won't for services where we pass auth in a header, for example. We're probably going to need a few different variants of this function for different auth methods (basic auth, header, query string, etc). I don't think that will be too hard - there's a few different ways we can do this.

I think I'd be fine just merging this how it is and saying at the moment it only works for querystring auth. Then we work out how to generalise this as we expand more services to use this helper. I usually find it easier to think about this stuff once I have a concrete problem I am trying to apply it to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See ffc7800, its all more generic now and should handle most service classes

.reply(200, dummyResponse)
expect(
await serviceClass.invoke(defaultContext, config, exampleInvokeParams),
).to.not.have.property('isError')
jNullj marked this conversation as resolved.
Show resolved Hide resolved

scope.done()
}

const defaultContext = { requestFetcher: fetch }

export { cleanUpNockAfterEach, noToken, defaultContext }
export {
cleanUpNockAfterEach,
noToken,
getBadgeExampleCall,
testAuth,
defaultContext,
}
Loading