Add routes for user activation #403

eddsalkield · 2020-12-01T19:36:22Z

Allow user accounts to be activated via an activation_callback, which is called in the /register route to handle user verification, resolving #106.

User accounts created through the /register route have is_active == True if and only if activation_callback is supplied to get_register_router.

The activation_callback expects a token, which if supplied to the /activate route will activate the user upon token verification.

The semantics of after_register have been changed slightly: it's called at the point when an activated user has been created. If no activation_callback is supplied, it's called after the /register route. Otherwise, it's called after the /activate route; then any desired behaviour to be run after /register should be put in the activation_callback.

This PR additionally:

Adds new error codes to fastapi_users/router/common.py
Update documentation
Add tests
Is backward-compatible with all previous function interfaces.

Co-authored-by: Mark Todd

Generate a token after creating the user in register route, passing to `activation_callback`, if `activation_callback` supplied Create new `/activate` route that will verify the token and activate the user Add new error codes to `fastapi_users/router/common.py` Update documentation Add tests Co-authored-by: Mark Todd <markpeter.todd@hotmail.co.uk>

frankie567 · 2020-12-02T06:08:06Z

Hi @eddsalkield!

Wow, that's quite some work, thank you :) I'll review that carefully!

codecov · 2020-12-02T06:14:39Z

Codecov Report

Merging #403 (b95f988) into master (7186554) will decrease coverage by 0.84%.
The diff coverage is 94.66%.

@@             Coverage Diff             @@
##            master     #403      +/-   ##
===========================================
- Coverage   100.00%   99.15%   -0.85%     
===========================================
  Files           22       23       +1     
  Lines          801      945     +144     
===========================================
+ Hits           801      937     +136     
- Misses           0        8       +8

Impacted Files	Coverage Δ
fastapi_users/user.py	`92.00% <86.66%> (-8.00%)`	⬇️
fastapi_users/router/verify.py	`92.85% <92.85%> (ø)`
fastapi_users/authentication/__init__.py	`100.00% <100.00%> (ø)`
fastapi_users/db/sqlalchemy.py	`100.00% <100.00%> (ø)`
fastapi_users/db/tortoise.py	`100.00% <100.00%> (ø)`
fastapi_users/fastapi_users.py	`100.00% <100.00%> (ø)`
fastapi_users/models.py	`100.00% <100.00%> (ø)`
fastapi_users/router/__init__.py	`100.00% <100.00%> (ø)`
fastapi_users/router/auth.py	`100.00% <100.00%> (ø)`
fastapi_users/router/common.py	`100.00% <100.00%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7186554...b95f988. Read the comment docs.

frankie567 · 2020-12-02T07:05:44Z

First of all, thank you for your work. It will be a very good starting point. I've some concerns on some points though, but that doesn't alter the quality of your code 🙂

Use of `is_active`

I see that you reused the is_active property to check if the user is activated or not. I'm not 100% sure about that.

On one hand, it's quite convenient and straightforward to understand.

On the other hand, it doesn't allow the distinction between an activated and a verified user:

An active user can login ; an inactive user cannot. The use case here is to be able to disable a user if they want to stop to use the app or if we want to suspend it for legal reasons.
A non-verified user should be able to login in my opinion. It's quite common in some apps to register and start to use it without having verified your e-mail. Usually, you have a banner on the top prompting you to do so ; and at some point, you'll hit a feature that requests you to be verified. Besides, we could for example ask the user to verify again if they change their email.

In such case, we would add a verified property to the User model and have a new dependency callable get_current_verified_user.

Still, I'm not sure about this, so I would be happy to discuss about it.

Activation logic in the register router

I would prefer to have the activation logic in a separate router ; just like we do for the reset password one. This way, it's way easier to test, remain fully optional and allow for developers to easily implement their own if they don't like the one we provide.

That means several things:

I don't think the default is_active value should be determined by the router. It should be set by the end-developer on its User model.
I prefer to have the after_register handler called whatever the case after the user is registered.
- We can always call it with an activation token, even if the app doesn't need one. Or, if we really care about that, have a parameter in get_register_route to state if we need it or not.
- The end-developer have the responsibility in its after_register handler to send the activation token.
The activation router would have an after_activation handler to do whatever the end-developer need (e.g. subscribe them to the onboarding marketing email, etc...)

I think the logic is clearer structured this way.

Other thoughts

In the create_user function, you raise the UserAlreadyExists exception only if the user is active. This won't work as we would try to insert another user with the same e-mail in the database ; which would conflict.
ACTIVATE_USER_LINK_USED should be called ACTIVATE_USER_ALREADY_ACTIVATED.

That's it 😅 I'm conscious that I'm asking you quite some rework ; so feel free to tell me if you prefer that I handle it from now 🙂

Before that, I would like to have your inputs (and also the ones of @lefnire) about the verified idea 😄

Cheers!

lefnire · 2020-12-02T07:20:46Z

No input here, y'all are the experts! Stoked about this though, props on the hard work

eddsalkield · 2020-12-02T16:25:47Z

Thanks for the swift response. We made a fair number of design decisions here, and so I'm more than happy to debate them to get it right.

Account verification

I now understand your intended semantics of activation - the example of deactivating a user account made this clear. Having had a think, I agree that we should separate activation from verification in two different routers. Doing so lets us keep the current implementation of after_register, introducing two new callbacks for the two new verification routes: one for after token generation to send an email to the user, and another for after_activation.

Ultimately, we need to let the end-developer answer the question: should a user have to be verified to log in? I suggest that we add a boolean argument to get_auth_router, called something like requires_verification for this.

To align with the semantics of verification, I think the default user model should not be verified by default. Therefore, the requires_verification argument of get_auth_router should also be False by default, preserving the current behaviour for unverified users.

To add verification, then end-developer then needs to:

Add the verification router
Supply requires_verification = True to get_auth_router

Your thoughts would be appreciated.

The verification router

The verification router will expose two routes: one for token generation, and one for verification using the token. Separating the token generation from the /register route allows us to support the flow of reverifying a user if their email changes, without making them register again.

Other thoughts

In the create_user function, you raise the UserAlreadyExists exception only if the user is active. This won't work as we would try to insert another user with the same e-mail in the database ; which would conflict.

This was required since token generation was tied to the /register route - to get another token (say, if the previous one expired) would require calling /register again, and so exceptions were not to be raised for created, although inactive, users. Bu keeping verification and activation separate, we avoid having to do this.

I think these changes shouldn't take too long to implement, so we'll have a crack at implementing them with your approval. Rewriting the docs and tests to match the new behaviour is likely going to take the longest!

frankie567 · 2020-12-02T17:19:55Z

Thanks! I feel we'll make great work there 😄

Ultimately, we need to let the end-developer answer the question: should a user have to be verified to log in? I suggest that we add a boolean argument to get_auth_router, called something like requires_verification for this.

To align with the semantics of verification, I think the default user model should not be verified by default. Therefore, the requires_verification argument of get_auth_router should also be False by default, preserving the current behaviour for unverified users.

Sounds great!

To add verification, then end-developer then needs to:

Supply requires_verification = True to get_auth_router

Or not, if he wants to allow to login even if not verified 😉

The verification router

The verification router will expose two routes: one for token generation, and one for verification using the token. Separating the token generation from the /register route allows us to support the flow of reverifying a user if their email changes, without making them register again.

Sounds perfect to me.

Let's make a quick to-do list then:

Add a is_verified boolean property on:
- BaseUser model
- BaseUserCreate model
- Exclude this property in the create_update_dict method of CreateUpdateDictModel. It ensures the end-user cannot set it itself while registering. A unit-test in test_router_register would be helpful there (similar to test_valid_body_is_superuser)
Create a verification router with two routes:
- /request (if you have a better naming idea, I'm open!): generates a token and call a after_verification_request handler
- /verify: checks token validity and set the is_verified flag to True on the user. Call a after_verification handler (should be optional IMO, as I don't think there is much to do at that point).
Add two new dependency callables to allow to protect the routes with verified user:
- get_optional_current_verified_user
- get_current_verified_user
- It happens in authentication/__init__.py. Things are a little bit scary there, so feel free to ask me if you are unsure.
Add an option on get_auth_router to let the end-developer decide if login is authorized for non-verified users.
Write the docs:
- I suggest you to wait that we are 100% happy with the implementation before starting

Are you okay with this? 🙂

eddsalkield · 2020-12-02T18:01:53Z

Good idea with the to-do list; it'll help us track what's left in the project.

However, on reflection, I think there are only two use cases we can support here:

No user verification, in which case accounts are given on a first come, first served basis. This is the current approach.
User verification, in which case accounts are only given once the user verifies that they are the owner of an email address.

I think that, from a security perspective, we cannot support a halfway house between these two. I’ll illustrate the problem through a short story.

Suppose that the end-developer configures verification, and allows users to log in and access a minimal set of site features without verifying. The site cannot make guarantees that the currently logged in user is the legitimate owner of the email address. In this case, a squatter could theoretically come along first and register an account. Now the legitimate owner needs to be able to come along later and oust the squatter. 😉

Let’s also suppose that the legitimate email address owner is unaware of the squatter’s existence, because the verification email got lost. Since the legitimate owner doesn't know the account password, or even know that the squatter exists, they would try to create a new account under their email address. There are two possibilities for what could occur:

The site responds saying “this email already registered”. This is a problem, because now the squatter can’t be evicted.
The site lets the legitimate owner create a new account that overwrites the old one. This also presents a problem, because the squatter can simply walk right back in, if they re-register before verification occurs.

In essence, having accounts that are first-come first-served until somebody gets a verification attempt in, is mad. I believe that either accounts should be given in the current free-for-all style, or restricted by means of verification, but without an intermediate possibility. If an end-developer wants to support a reduced feature set for certain users, they should implement it as an additional permissions layer atop FastAPI-Users, having decided whether to support verification or not beforehand.

However, is_active and is_verified should remain separate because they encapsulate separate semantics: is_active allows the user to be banned later on, whereas is_verified communicates whether the current email address is owned by this user.

Otherwise, I'm happy with the rest of the plan, and would like to hear your thoughts about how to proceed on this.

lefnire · 2020-12-02T18:04:29Z

The squatted could do a forgot-password, since they're the email owner, and get into the squatters account, then delete. Kinda a pain, but just a thought

lefnire · 2020-12-02T18:06:43Z

They'll also know immediately they got squatted by getting an email-verification from a service they've never heard of, and can investigate what's up

frankie567 · 2020-12-02T20:33:43Z

I agree with @lefnire here. Given that the squatted user keeps the control on their mailbox, they can always ask for a reset password email. As I see it, the potential benefits for the attacker seem quite limited.

I've tested to create an account on GitHub and Gitlab :

GitHub allows you to login and access personal settings. You can't create a repository or participate in discussions until you verify your email.
Gitlab allows you to do everything, except use the CI features.

So, I would say that it's perfectly okay to allow login and some actions for non-verified users (typically, you at least need to allow it for verification routes).

If the end-developer doesn't want to allow any action without verification, they would still be able to do so.

What do you think?

mark-todd · 2020-12-04T11:40:41Z

Hey folks,

Just a quick update - yep, all of that is fine Frankie - Edd and I have implemented all of the changes on the checklist and are just putting together the tests before making another pull request. We should have it ready at some point in the next few days. :)

Kind regards,

Mark

mark-todd · 2020-12-11T10:41:09Z

Just thought I'd give you all another quick update - the tests and changes are now complete, we're just doing a review before we send it in, so the next pull request should be ready in the next couple of days :)

frankie567 · 2020-12-11T10:52:00Z

That's great @mark-todd! Thank you for the update!

* Separate verification logic and token generation into `/fastapi_users/router/verify.py`, with per-route callbacks for custom behaviour * Return register router to original state * Added `is_verified` property to user models * Added `requires_verification` argument to `get_users_router`and `get_auth_router` * Additional dependencies added for verification in `fastapi_users/authentication/__init__.py` * Update tests for new behaviour * Update `README.md` to describe a workaround for possible problems during testing, by exceeding ulimit file descriptor limit Co-authored-by: Mark Todd <markpeter.todd@hotmail.co.uk>

eddsalkield · 2020-12-14T19:25:23Z

We've implemented the changes and corresponding tests in our latest commit. I'm glad we've been able to iterate a bit to converge on this design, which I think is good overall.

Feedback appreciated!

frankie567

Hey there!

First of all, I wanted to thank you for your work! This is quite a big thing!

I've made some comments and questions. When we have solved that, I'll merge that PR. Then, I'll probably revisit some things (I admit I've not read all those unit tests) before release.

Cheers guys 🎄

frankie567 · 2020-12-23T14:50:11Z

fastapi_users/router/verify.py

+):
+    router = APIRouter()
+
+    @router.post("/request_verify_token", status_code=status.HTTP_202_ACCEPTED)


I prefer kebab-case for API paths (with "-")

Sure, we can make it consistent with this

frankie567 · 2020-12-23T14:53:23Z

fastapi_users/user.py

    ) -> models.BaseUserDB:
        existing_user = await user_db.get_by_email(user.email)

-        if existing_user is not None:
+        if existing_user is not None and existing_user.is_active:


Still not convinced by this. I would prefer to remove it ; and maybe talk about it in a separate issue/PR.

Ok, I suppose without it it just means inactive users get a nicer error message upon re-registration - I think we just added this so that if you're banned from a site you don't get the nice "UserAlreadyExists" flag, as this could be potentially useful information to malicious users. That said, I'm not sure it's that big a deal, so I'm happy to revert it :)

frankie567 · 2020-12-23T14:55:45Z

fastapi_users/user.py

            raise UserAlreadyExists()

        hashed_password = get_password_hash(user.password)
        user_dict = (
            user.create_update_dict() if safe else user.create_update_dict_superuser()
        )
+        if is_active is not None:


This is not necessary as it should be handled properly by create_update_dict and create_update_dict_superuser.

Excellent! Yeah I think I just accidentally implemented this twice haha - the changes are already made to the create_update_dicts

frankie567 · 2020-12-23T14:55:53Z

fastapi_users/user.py

            raise UserAlreadyExists()

        hashed_password = get_password_hash(user.password)
        user_dict = (
            user.create_update_dict() if safe else user.create_update_dict_superuser()
        )
+        if is_active is not None:
+            user_dict["is_active"] = is_active
+        if is_verified is not None:


This is not necessary as it should be handled properly by create_update_dict and create_update_dict_superuser.

Excellent! Yeah I think I just accidentally implemented this twice haha - the changes are already made to the create_update_dicts

frankie567 · 2020-12-23T14:57:53Z

fastapi_users/user.py

+
+
+class VerifyUserProtocol(Protocol):
+    def __call__(self, user_uuid: str) -> Awaitable[models.BaseUserDB]:


The type annotation for user_uuid should be UUID4 (from pydantic)

Ah ok thanks!

frankie567 · 2020-12-23T15:01:47Z

tests/conftest.py

+@pytest.fixture
+def verified_user() -> UserDB:
+    return UserDB(
+        email="lake.lady@camelot.bt",


Ha, ha, you got the spirit 😄 Nice 👍

frankie567 · 2020-12-23T15:03:27Z

tests/test_fastapi_users.py

+    app.include_router(
+        fastapi_users.get_verify_router(
+            after_verification_request=verification_callback,
+            verification_token_secret="teststring",


Let's use "SECRET" to be consistent with the others.

frankie567 · 2020-12-23T15:07:40Z

docs/configuration/model.md

@@ -4,7 +4,7 @@



We should revert those changes. (except grammar fixes of courses)

Yep, agreed

frankie567 · 2020-12-23T15:07:48Z

docs/configuration/routers/register.md

@@ -1,9 +1,26 @@
 # Register routes


We should revert those changes. (except grammar fixes of courses)

Yep, agreed

frankie567 · 2020-12-23T15:08:16Z

docs/usage/routes.md

@@ -44,7 +44,7 @@ Logout the authenticated user against the method named `name`. Check the corresp



We should revert those changes.

Yep, agreed

mark-todd

Hi Frankie

Yep this all seems fine, I think the only potential question mark now is the get_get_user function - if you're happy with my explanation though I can go through and make the required changes. :)

Cheers,
Mark

fastapi_users/user.py

mark-todd · 2021-01-04T10:19:53Z

fastapi_users/user.py

+    user_db: BaseUserDatabase[models.BaseUserDB],
+) -> GetUserProtocol:
+    async def get_user(user_email: EmailStr) -> models.BaseUserDB:
+        if not (user_email == EmailStr(user_email)):


get_user is the equivalent of create_user, only it simply fetches and returns the user without adding them to the database. It was required to abstract away from accessing the database directly in verify.py in an analogous way to your abstraction in register.py for create_user, although it would also be a useful function for future sections of fastapi-users.

mark-todd · 2021-01-04T10:21:55Z

fastapi_users/router/verify.py

+):
+    router = APIRouter()
+
+    @router.post("/request_verify_token", status_code=status.HTTP_202_ACCEPTED)


Sure, we can make it consistent with this

mark-todd · 2021-01-04T10:22:43Z

fastapi_users/user.py

+
+
+class VerifyUserProtocol(Protocol):
+    def __call__(self, user_uuid: str) -> Awaitable[models.BaseUserDB]:


Ah ok thanks!

mark-todd · 2021-01-04T10:24:01Z

fastapi_users/user.py

            raise UserAlreadyExists()

        hashed_password = get_password_hash(user.password)
        user_dict = (
            user.create_update_dict() if safe else user.create_update_dict_superuser()
        )
+        if is_active is not None:


Excellent! Yeah I think I just accidentally implemented this twice haha - the changes are already made to the create_update_dicts

mark-todd · 2021-01-04T10:25:11Z

docs/configuration/model.md

@@ -4,7 +4,7 @@



Yep, agreed

mark-todd · 2021-01-04T10:25:17Z

docs/configuration/routers/register.md

@@ -1,9 +1,26 @@
 # Register routes


Yep, agreed

mark-todd · 2021-01-04T10:25:25Z

docs/usage/routes.md

@@ -44,7 +44,7 @@ Logout the authenticated user against the method named `name`. Check the corresp



Yep, agreed

mark-todd · 2021-01-04T10:43:55Z

fastapi_users/user.py

    ) -> models.BaseUserDB:
        existing_user = await user_db.get_by_email(user.email)

-        if existing_user is not None:
+        if existing_user is not None and existing_user.is_active:


Ok, I suppose without it it just means inactive users get a nicer error message upon re-registration - I think we just added this so that if you're banned from a site you don't get the nice "UserAlreadyExists" flag, as this could be potentially useful information to malicious users. That said, I'm not sure it's that big a deal, so I'm happy to revert it :)

mark-todd · 2021-01-04T10:45:46Z

tests/test_fastapi_users.py

+    app.include_router(
+        fastapi_users.get_verify_router(
+            after_verification_request=verification_callback,
+            verification_token_secret="teststring",


Kebab-case on request-verify-token SECRET now used as test string Other minor changes

Add requested modifications for user verification.

frankie567 · 2021-01-09T08:24:04Z

Many thanks for your great work @mark-todd and @eddsalkield 👍 I've merged it in a sub-branch ; I'll take the liberty to revisit it before putting it in master!

lefnire · 2021-01-09T21:52:03Z

Nice job y'all! Epic epic

* Add routes for user activation (#403) * Add routes for user activation Generate a token after creating the user in register route, passing to `activation_callback`, if `activation_callback` supplied Create new `/activate` route that will verify the token and activate the user Add new error codes to `fastapi_users/router/common.py` Update documentation Add tests Co-authored-by: Mark Todd <markpeter.todd@hotmail.co.uk> * Rework routes for user activation * Separate verification logic and token generation into `/fastapi_users/router/verify.py`, with per-route callbacks for custom behaviour * Return register router to original state * Added `is_verified` property to user models * Added `requires_verification` argument to `get_users_router`and `get_auth_router` * Additional dependencies added for verification in `fastapi_users/authentication/__init__.py` * Update tests for new behaviour * Update `README.md` to describe a workaround for possible problems during testing, by exceeding ulimit file descriptor limit Co-authored-by: Mark Todd <markpeter.todd@hotmail.co.uk> * Restored docs to original state. * All other modifications reqested added Kebab-case on request-verify-token SECRET now used as test string Other minor changes Co-authored-by: Mark Todd <markpeter.todd@hotmail.co.uk> * Embed token in body in verify route * Reorganize checks in verify route and add unit test * Ignore coverage on Protocol classes * Tweak verify_user function to take full user in parameter * Improve unit tests structure regarding parametrized test client * Make after_verification_request optional to be more consistent with other routers * Tweak status codes on verify routes * Write documentation for verification feature * Add not released warning on verify docs Co-authored-by: Edd Salkield <edd@salkield.uk> Co-authored-by: Mark Todd <markpeter.todd@hotmail.co.uk>

frankie567 · 2021-02-06T17:19:06Z

@all-contributors add @eddsalkield for code, doc
@all-contributors add @mark-todd for code, doc

allcontributors · 2021-02-06T17:19:07Z

@frankie567

I could not determine your intention.

Basic usage: @all-contributors please add @Someone for code, doc and infra

For other usages see the documentation

frankie567 · 2021-02-06T17:19:15Z

@all-contributors add @mark-todd for code, doc

allcontributors · 2021-02-06T17:19:22Z

@frankie567

I've put up a pull request to add @mark-todd! 🎉

frankie567 · 2021-02-06T17:20:52Z

@all-contributors add @eddsalkield for code, doc

allcontributors · 2021-02-06T17:21:00Z

@frankie567

I've put up a pull request to add @eddsalkield! 🎉

frankie567 · 2021-02-06T17:21:31Z

@all-contributors add @mark-todd for code, doc

allcontributors · 2021-02-06T17:21:38Z

@frankie567

I've put up a pull request to add @mark-todd! 🎉

eddsalkield mentioned this pull request Dec 1, 2020

Accounts: switch to AWS Cognito or SuperTokens? ocdevel/gnothi#107

Closed

5 tasks

frankie567 added the enhancement New feature or request label Dec 2, 2020

frankie567 linked an issue Dec 3, 2020 that may be closed by this pull request

Routes for email verification #106

Closed

frankie567 requested changes Dec 23, 2020

View reviewed changes

mark-todd reviewed Jan 4, 2021

View reviewed changes

mark-todd and others added 3 commits January 4, 2021 12:19

Restored docs to original state.

ba9a732

All other modifications reqested added

d0158d6

Kebab-case on request-verify-token SECRET now used as test string Other minor changes

Merge pull request #4 from mark-todd/user_activation

b95f988

Add requested modifications for user verification.

frankie567 changed the base branch from master to user-activation January 9, 2021 08:21

frankie567 merged commit b756785 into fastapi-users:user-activation Jan 9, 2021

allcontributors bot mentioned this pull request Feb 6, 2021

docs: add mark-todd as a contributor #484

Closed

allcontributors bot mentioned this pull request Feb 6, 2021

docs: add eddsalkield as a contributor #485

Merged

allcontributors bot mentioned this pull request Feb 6, 2021

docs: add mark-todd as a contributor #486

Merged



		class VerifyUserProtocol(Protocol):
		def __call__(self, user_uuid: str) -> Awaitable[models.BaseUserDB]:

		@@ -44,7 +44,7 @@ Logout the authenticated user against the method named `name`. Check the corresp

Add routes for user activation #403

Add routes for user activation #403

Conversation

eddsalkield commented Dec 1, 2020

frankie567 commented Dec 2, 2020

codecov bot commented Dec 2, 2020 • edited

Codecov Report

frankie567 commented Dec 2, 2020

Use of is_active

Activation logic in the register router

Other thoughts

lefnire commented Dec 2, 2020

eddsalkield commented Dec 2, 2020

Account verification

The verification router

Other thoughts

frankie567 commented Dec 2, 2020

The verification router

Let's make a quick to-do list then:

eddsalkield commented Dec 2, 2020

lefnire commented Dec 2, 2020

lefnire commented Dec 2, 2020

frankie567 commented Dec 2, 2020

mark-todd commented Dec 4, 2020

mark-todd commented Dec 11, 2020

frankie567 commented Dec 11, 2020

eddsalkield commented Dec 14, 2020

frankie567 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mark-todd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frankie567 commented Jan 9, 2021

lefnire commented Jan 9, 2021

frankie567 commented Feb 6, 2021

allcontributors bot commented Feb 6, 2021

frankie567 commented Feb 6, 2021

allcontributors bot commented Feb 6, 2021

frankie567 commented Feb 6, 2021

allcontributors bot commented Feb 6, 2021

frankie567 commented Feb 6, 2021

allcontributors bot commented Feb 6, 2021

codecov bot commented Dec 2, 2020 •

edited

Use of `is_active`