Skip to content

Conversation

@jNullj
Copy link
Member

@jNullj jNullj commented Feb 22, 2024

This PR is follow-up for #9681 and helps to reach the goal of #9493

This PR will include refactor of all basicAuth services:

Additional changes to authTest:
4d57607 - Add exampleOverride
f156762 - Add authOverride
cd6c65b - Add configOverrid
cf34fae - Split invoke params into path and query
a713669 - Improve error handling, Throw error when service auth key is missing
99a01e1 - Handle case of userKey without passKey
74554d7 - Add option to ignore openApi example in testAuth
8c38355 - Handle auth.defaultToEmptyStringForUser
57163eb - Add support for multiple requests in service

jNullj added 30 commits October 24, 2023 00:08
Change auth tests to include all shields of the base class.
The code is formated to be used in more general cases and increases code reuseability.
We already test all existing classes, no need for a dummy
Add getBadgeExampleCall to extract the first OpenAPI example then reformat it for service invoke function.
Add the testAuth function which tests auth of a service (badge) using a provided dummy response.
Add all auth methods used to testAuth to be generic and used by all services.
Add helper functions to make testAuth more readable
Use apiHeaderKey & bearerHeaderKey as function params rather then extracting them with regex from function strings.

Those options are now part of an options object param joined with the contentType that replaces header.

header was originaly added for setting content type of the reply, so it makes more sense to directly set the content type
Before this commit the QueryStringAuth would only work for the key of stackexchange.
This commit makes the testAuth function generic and allows passing user and pass keys.
Might set wrong header for jwt login request.
This commit fixes that.
Some services might have more then one authOrigin.
This commit makes sure we test for redundent authOrigins as well as support requests to them if needed.
Prior to this change, JwtAuth testing would lead to erros due to the absence of a specified login endpoint,
Nock would be dumplicated for both login and non login hosts and indicate a missing request.

This commit enforces the requirement for a new jwtLoginEndpoint argument when testing JwtAuth.
The argument seperates the endpoint nock scope from the behavior of the request nock.
jNullj added 2 commits October 3, 2024 15:19
Example changed at 62ed7c3
The new example schema differ from old one
This caused test to fail
Update example response to fit new schema
remove for adding these changes in a later PR.
this test adds the authOverride we try to get rid of.
This blocks testAuth development and is planned for a later time.
jNullj added a commit to jNullj/shields-fun-fork that referenced this pull request Oct 3, 2024
part of Improve our approach for testing auth badges#9493
seperated from PR badges#9983 for work in a later time
@jNullj
Copy link
Member Author

jNullj commented Oct 3, 2024

I've been working 12-hour days recently, making it hard to complete tasks. With the holidays, I found time to finish this work.

Future PRs will be smaller to avoid bottlenecks.

I've removed Bitbucket as suggested and moved those changes to a separate branch. Although authOverride is currently unused, I've kept it in case we need it for other services until a better solution is found.

@jNullj jNullj marked this pull request as ready for review October 3, 2024 13:49
@jNullj jNullj requested a review from chris48s October 3, 2024 13:50
@PyvesB
Copy link
Member

PyvesB commented Sep 27, 2025

@jNullj shall we try to land this one? If so, could you please update it with latest revision, and I'll give it a look in the coming weeks. Worth noting that I added auth support for Reddit in #10790, and maybe other services were updated as well. This PR may require some additional changes.

Although authOverride is currently unused, I've kept it in case we need it for other services until a better solution is found.

In general, my philosophy is to keep things simple until we really need something. Especially in a world where so few people can dedicate time to the project. But up to you :)

@jNullj
Copy link
Member Author

jNullj commented Sep 28, 2025

I prefer to cut the work into smaller chunks.
Can we stick to existing service tests? I will scan for missing ones and add them in part 3, just to keep these PR more manageable.
I will keep the function until all services are converted in case we need it, but i agree that we should not keep unused code, so once we reach the end of this change i will remove unused functions as well.

@PyvesB
Copy link
Member

PyvesB commented Sep 28, 2025

Works for me. Will give this a proper review in the coming days.

Copy link
Member

@PyvesB PyvesB left a comment

Choose a reason for hiding this comment

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

This is starting to shape up nicely! Noticed a few minor issues whilst reading through the changes.

jNullj and others added 4 commits January 10, 2026 22:18
Co-authored-by: Pierre-Yves Bigourdan <10694593+PyvesB@users.noreply.github.com>
Co-authored-by: Pierre-Yves Bigourdan <10694593+PyvesB@users.noreply.github.com>
Co-authored-by: Pierre-Yves Bigourdan <10694593+PyvesB@users.noreply.github.com>
@jNullj
Copy link
Member Author

jNullj commented Jan 10, 2026

wops, sorry for the ping guys, i will fix that in a bit

@jNullj jNullj force-pushed the feat/9493/improve-auth-testing branch from 506e93b to ed23542 Compare January 10, 2026 20:29
Was outdated after merging with master.
This commit fixes origin test to openapi example.
Avoid exact body match requirement.
We are only intrested in username and password values to match, not exact one to one object.
@jNullj jNullj requested a review from PyvesB January 10, 2026 21:26
@jNullj
Copy link
Member Author

jNullj commented Jan 10, 2026

Thank you for the review, i fixed all comments, also found some additional things to fix.
Merged master to keep this up to date.

@jNullj jNullj added this pull request to the merge queue Jan 11, 2026
Merged via the queue into badges:master with commit fcfbf89 Jan 11, 2026
19 checks passed
@jNullj jNullj deleted the feat/9493/improve-auth-testing branch January 11, 2026 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Server, BaseService, GitHub auth, Shared helpers developer-experience Dev tooling, test framework, and CI javascript [dependabot only] Pull requests that update Javascript packages

Development

Successfully merging this pull request may close these issues.

3 participants