-
Notifications
You must be signed in to change notification settings - Fork 43
Add multifactorauth setup #520
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 multifactorauth setup #520
Conversation
supertokens_python/recipe/multifactorauth/multi_factor_auth_claim.py
Outdated
Show resolved
Hide resolved
supertokens_python/recipe/multifactorauth/multi_factor_auth_claim.py
Outdated
Show resolved
Hide resolved
supertokens_python/recipe/multifactorauth/multi_factor_auth_claim.py
Outdated
Show resolved
Hide resolved
supertokens_python/recipe/multifactorauth/multi_factor_auth_claim.py
Outdated
Show resolved
Hide resolved
class ValidFirstFactorResponse: | ||
def __init__( | ||
self, | ||
status: Literal["OK", "INVALID_FIRST_FACTOR_ERROR", "TENANT_NOT_FOUND_ERROR"], | ||
) -> None: | ||
self.status = status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these need to be individual classes. Otherwise there is no point in having this class as you can just return Literal directly anyway
required_secondary_factors: Optional[List[str]] = None, | ||
first_factors: Optional[List[str]] = None, | ||
): | ||
self.emailpassword = emailpassword | ||
self.passwordless = passwordless | ||
self.third_party = third_party | ||
self.core_config = core_config | ||
self.required_secondary_factors = required_secondary_factors | ||
self.first_factors = first_factors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this change required here for this PR?
update_and_get_mfa_related_info_in_session, | ||
) | ||
from supertokens_python.recipe.session import SessionContainer | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need these changes here for this pr?
|
||
from supertokens_python.async_to_sync_wrapper import sync | ||
from supertokens_python.recipe.session import SessionContainer | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need these changes here for this pr?
options: APIOptions, | ||
session: SessionContainer, | ||
user_context: Dict[str, Any], | ||
) -> Optional[Dict[str, object]]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a badly typed output!
tenant_info = await get_tenant(tenant_id, input.user_context) | ||
return ( | ||
tenant_info.required_secondary_factors | ||
if tenant_info and tenant_info.required_secondary_factors | ||
else [] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to throw unauthorised error if tenant is None
factors_set_up_for_user=asyncio.create_task(get_factors_setup_for_user()), | ||
required_secondary_factors_for_user=asyncio.create_task( | ||
get_required_secondary_factors_for_user() | ||
), | ||
required_secondary_factors_for_tenant=asyncio.create_task( | ||
get_required_secondary_factors_for_tenant() | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is all wrong. We can't use create_task just like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The input type of get_mfa_requirements_for_auth needs to change to take functions!
class FactorsResponse(TypedDict): | ||
next: list[str] | ||
already_setup: list[str] | ||
allowed_to_setup: list[str] | ||
|
||
|
||
class ResyncSessionResponse(TypedDict): | ||
status: str | ||
factors: FactorsResponse | ||
emails: Dict[str, Optional[list[str]]] | ||
phone_numbers: Dict[str, Optional[list[str]]] | ||
|
||
|
||
ResyncSessionResult = ResyncSessionResponse | GeneralErrorResponse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these dont seem to be used anywhere in the pr. Hence they are not needed here
functions: Optional[Callable[[RecipeInterface], RecipeInterface]], | ||
apis: Optional[Callable[[APIInterface], APIInterface]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is incorrectly typed. It should have None by default!
@@ -11,7 +11,7 @@ | |||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | |||
# License for the specific language governing permissions and limitations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related to this file, but there are a lot of issues from a compiling / linting point of view. Run make check-lint
Summary of change
(A few sentences about this PR)
Related issues
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Documentation changes
(If relevant, please create a PR in our docs repo, or create a checklist here highlighting the necessary changes)
Checklist for important updates
coreDriverInterfaceSupported.json
file has been updated (if needed)supertokens_python/constants.py
frontendDriverInterfaceSupported.json
file has been updated (if needed)setup.py
supertokens_python/constants.py
git tag
) in the formatvX.Y.Z
, and then find the latest branch (git branch --all
) whoseX.Y
is greater than the latest released tag.supertokens_python/utils.py
file to include that in theFRAMEWORKS
variablesyncio
/asyncio
functions are consistent.tests/sessions/test_access_token_version.py
to account for any new claims that are optional or omitted by the coreRemaining TODOs for this PR