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

Add support for AuthSession cookie #617

Merged
merged 14 commits into from
Jun 20, 2024
Merged

Add support for AuthSession cookie #617

merged 14 commits into from
Jun 20, 2024

Conversation

paulpascal
Copy link
Contributor

@paulpascal paulpascal commented May 18, 2024

Description

This PR introduces support for a session token-based authentication mechanism in cht-conf, supporting the User Management Tool's cloud-based move-contact execution feature.

Key changes

  • New command argument: Added support for new cmdArgs, session-token, allowing API requests and database operations to use a session cookie (if provided) for authentication instead of the regular basic authentication.

  • API requests: Updated API request functions to include the session token in the request headers if provided.

  • Database Authentication: Updated the db connection setup to utilize the session token for authentication using pouchdb-session-authentication plugin.

Note

With these changes, the User Management Tool can rely on a worker to run the cht commands in a child process, passing along the sessionToken obtained initially from the _session request.
Please find more details here: PR, issue.

@paulpascal paulpascal changed the title Add support for AuthSession cookie for login Add support for AuthSession cookie May 18, 2024
@garethbowen
Copy link
Member

Related to #582

@dianabarsan Can you please have a look at this when you get a chance? How does this work with the pouchdb cookie auth plugin I wonder...

@dianabarsan
Copy link
Member

dianabarsan commented May 20, 2024

I think @paulpascal could just use the PouchDb session plugin and drop all the custom code.

Ah, the session is passed as an argument to the code? I suppose we could have a version of the plugin that can get a session cookie as a parameter when creating the database and have it use that cookie throughout.

@garethbowen
Copy link
Member

Ah, the session is passed as an argument to the code?

Yeah I made that mistake on my first read through too!

I suppose we could have a version of the plugin...

Yes I think that makes sense... if you have a session token passed in, use that, otherwise use the basic auth.

@dianabarsan
Copy link
Member

dianabarsan commented May 20, 2024

Yea, the intent of the plugin is to not need to add custom code to handle sessions everywhere. So it'd be pretty counterproductive to have each repo have its custom implementation.

@paulpascal can we sync on what is the need here and how can we use https://github.com/medic/pouchdb-session-authentication , which we intend to support, maintain and eventually integrate into PouchDb core?

@paulpascal
Copy link
Contributor Author

Thanks @garethbowen and @dianabarsan for looking into this.

@dianabarsan sure we can sync on that 👌.

Copy link
Member

@dianabarsan dianabarsan left a comment

Choose a reason for hiding this comment

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

I think this looks great!

However I think we should have some e2e tests that check that cht-conf works correctly when a session token is passed. The tests should cover api requests and pouchdb requests.

@paulpascal
Copy link
Contributor Author

I think this looks great!

However I think we should have some e2e tests that check that cht-conf works correctly when a session token is passed. The tests should cover api requests and pouchdb requests.

Sure, let me add that 👍

Copy link
Member

@dianabarsan dianabarsan left a comment

Choose a reason for hiding this comment

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

Very very nice! Very high quality, I am so happy when I see PRs like this.
I have just some really minor comments.

src/lib/db.js Outdated Show resolved Hide resolved
test/e2e/session-token.spec.js Outdated Show resolved Hide resolved
test/e2e/session-token.spec.js Outdated Show resolved Hide resolved
Copy link
Member

@dianabarsan dianabarsan left a comment

Choose a reason for hiding this comment

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

Loving this!

.to.be.rejected
.and.eventually.have.property('message')
// Bad Request: Malformed AuthSession cookie
.that.contains('INFO Error: Received error code 400');
Copy link
Member

Choose a reason for hiding this comment

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

<3

@paulpascal
Copy link
Contributor Author

Thanks @dianabarsan !

@paulpascal paulpascal merged commit efbc9cf into main Jun 20, 2024
14 checks passed
@paulpascal paulpascal deleted the session-token-auth branch June 20, 2024 13:54
@medic-ci
Copy link
Collaborator

🎉 This PR is included in version 3.23.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants