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

test: Create IdP registration integration test #669

Merged
merged 1 commit into from
Mar 24, 2021

Conversation

joachimvh
Copy link
Member

PR is not finished yet, but can already give an idea on how the integration tests for the IdP would work.

The panva oidc library does not work without cookies, so during the tests they have to be parsed and passed along.

Since our server only returns html, some html parsing is done to get the correct values needed for the next step. This does indicate that it would be possible in the future to support an API-based interface (still need the cookie headers though).

Currently get the warning below when running the tests. It's from the solid-client-authn-js library so if someone knows how to make sure it doesn't appear anymore please let me know.

DraftWarning: The DPoP APIs implements an IETF draft. Breaking draft implementations are included as minor versions of the openid-client library, therefore, the ~ semver operator should be used and close attention be payed to library changelog as well as the drafts themselves.

Next step is to add more steps in the step, specifically using the IdP for authentication and also invalid requests. Besides that some more cleaning up and potentially simplification.

@joachimvh joachimvh force-pushed the test/idp-integration branch from 2bdf9b8 to 36236d9 Compare March 19, 2021 15:59
@joachimvh
Copy link
Member Author

I'm currently having a very weird issue with this. Not even a clue why this is happening:

Currently all the tests in the file are succeeding, except the last one which tries to access a resource on the server when logged in. The reason that one fails is because the DPoPWebIdExtractor extracts the credentials as being http://alice.example/card#me. My guess is that the reason it does this is because jest uses the mocked verifier function in the __mocks__ folder.

Now, the problem is that if I delete that file (because I just wanted to quickly test it without the mock) suddenly all tests in this file fail with the following error:

Error: Failed to get module element SuffixAuxiliaryIdentifierStrategy from module @solid/community-server

The reason I'm guessing specifically this class is mentioned is because it's the first one being used in config/presets/acl.json, which is the first one being imported in the config used here. Starting the server normally with npm start still works fine, even though that uses pretty much the same config as this test suite here (except that one goes through CliRunner).

But I have no idea at all how removing the mock file could trigger this. Going to look more into this next week. Here's hoping it's just something silly somewhere that's easy to fix. Any suggestions what could cause this are welcome.

/cc @rubensworks @RubenVerborgh

@rubensworks
Copy link
Contributor

Such an error might indicate (and hide) some other npm module-level error.

You could try to print the error that's produced here: https://github.com/LinkedSoftwareDependencies/Components.js/blob/master/lib/construction/strategy/ConstructionStrategyCommonJs.ts#L40-L52
(We may even want to consider not swallowing the error there)

@joachimvh
Copy link
Member Author

Update: was the same issue as matthieubosquet/ts-dpop#13, so we'll also have to include the same mapper in the jest config.

@joachimvh joachimvh force-pushed the test/idp-integration branch from 36236d9 to c2421ce Compare March 22, 2021 15:14
@joachimvh joachimvh marked this pull request as ready for review March 22, 2021 15:15
@joachimvh
Copy link
Member Author

Test is finished and contains the following steps:

  • registration
  • logging in and accessing data that requires authorization
  • resetting a password
  • logging in with the new password

I think this covers all the major cases of the idp? I also tried to abstract away some of the calls that are the same for every scenario. I don't think there's a (clean) way to disable the DraftWarning so will probably have to leave this as it is currently.

@RubenVerborgh
Copy link
Member

Mock DraftWarning?

@joachimvh
Copy link
Member Author

Mock DraftWarning?

It's actually not an object that's made but an event that's emitted on process: https://github.com/panva/node-openid-client/blob/a079aee7af085373d4e873003032cfc44e94a51c/lib/client.js#L1651

@RubenVerborgh
Copy link
Member

We could mock process.emitWarning.

@joachimvh joachimvh force-pushed the test/idp-integration branch from c2421ce to 34c02ce Compare March 23, 2021 08:47
@joachimvh
Copy link
Member Author

We could mock process.emitWarning.

Good idea, did that.

Copy link
Contributor

@matthieubosquet matthieubosquet left a comment

Choose a reason for hiding this comment

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

LGTM

@joachimvh joachimvh merged commit e89c899 into feat/identity-provider-refactor Mar 24, 2021
@joachimvh joachimvh deleted the test/idp-integration branch March 24, 2021 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants