• for local dev
• setup circleci testing

typo in ResetPasswordFeature

• Implement the remaining tests for signin/up
• Make the test script more robust, fails sometimes with shadowRoot error.
• Add CircleCI configs

TODO in the code to add relevant links to documentations.

If I go to /auth/hello, I see a white screen. Instead, we should maybe redirect the user to /? Or to /auth (or whatever the base path is as specified by the user)

• import Session from "supertokens-auth-react/recipe/session"
• Is a wrapper around supertokens/supertokens-website. Therefore, this provides all the functions that supertokens-website provides.
• Must follow its respective part from the new FDI (v1.3.0)
• The init function

Below the sign up form, it is written:
"By signin up, you agree to ourTerms of Service and Privacy Policy"

• There is a typo in "signing"
• We want to show the terms of services and privacy policy only if they provide a link to those in the init function. If they provide a link to:
  • None: We do not want this sentence
  • Any one: We want just that mentioned in the sentence
    • "By signing up, you agree to our Terms of Service" or
    • "By signing up, you agree to our Privacy Policy"
  • Both:
    • "By signing up, you agree to our Terms of Service and Privacy Policy"

• import { ResetPasswordUsingToken } from "supertokens-auth-react/recipe/emailpassword"
• The default implementation should be in ${websiteBasePath}/reset-password
• The default theme should be ResetPasswordUsingTokenTheme
• Props for this component (all optional, all must have a default implementation)
• Will pass the following down to the Theme component (along with whatever the user passes):
  • submitNewPasswordForm Object
  • enterEmailForm Object
• They must be responsible to render a shadow-root to prevent CSS clashes
• Should have its own config in the init function as explained in this comment
• If a session exists already and the user visits ${websiteBasePath}/reset-password, with a token in the URL, then continue the flow normally.
• If the user visit ${websiteBasePath}/reset-password without a token in the URL, the enter email form is showed, otherwise the submit new password form is showed
• With a token, then URL format will be ${websiteBasePath}/reset-password?token=...&rid=email-password

Our theme right now supports only string values, but if someone makes their own theme, that may have other input values too. An input type of a number / boolean etc.. should be allowed in the form fields array when sending to the backend.

Edit:

To allow for this on the backend, I had to make the following changes (pick whichever apply on the frontend):

• Changing types of the validate function to make the type of value from string to any
• Changing types of the handleCustomFields callback post sign up API (not relevant to the frontend)
• Changing default password and email validators to first check that the input is of type string

Here is the backend commit with the above changes for your reference: supertokens/supertokens-node@20691af

Generic StyleProvider taking a getDefaultStyle à sprops, move library to components/common for use in other themes from other repos.

• import { ResetPasswordUsingTokenTheme } from "supertokens-auth-react/recipe/emailpassword"
• Themes should use a ThemeProvider which can be used to overwrite the default theme based on the passed props.

• import EmailPassword from "supertokens-auth-react/recipe/emailpassword"

• All features in this module will have a default full page layout, and a widget component that the user can use to add that anywhere on their website. The full page layouts must use these same widget components.

• The default full page of all features must be able to be disabled via a disableDefaultImplementation boolean passed to them in their init function.

• All features must follow their respective parts of the new FDI (v1.3.0)

• Following are the sets of feature components:

  • SignInAndUp
  • ResetPasswordUsingToken

• The init function of this module takes options as per the above features:

  EmailPassword.init({
     palette: {...},
     useShadowDom: false,
     signInAndUpFeature: {...},
     resetPasswordUsingTokenFeature: {...}
  })
  

• palette represents the styling theme (primary colours, secondary colours, link, background colours ...)

• useShadowDom: Represents wether we should use the shadow root dom to encapsulate our components. Pros and cons are discribed in #22

This might affect how error handling of queries to the API behaves.

To see fetch behaviour, please run the following on any browser:

fetch("https://httpstat.us/500")
    .then(function(response) {
        console.log("error not thrown", response.status);
    })

And the output of the above will be "error not thrown 500"

It seems that the login box's height and width takes precedence over other item's height and width.

If you run the demo app and go from sign in to reset password UI, you will see that the footer size increases. That is because the sign in form is squashing the footer it seems.

I am not sure about this issue since it could be something wrong with my CSS

package-lock's purpose is to generate the exact same dependencies tree wherever the package is installed.
It is intended to be commited. See Stackoverflow

Any reason why we are not commiting it atm?

Should be responsible for:

• Provide routing functionality. The algorithm is here.

• Has a way for any module to get the appInfo if needed.

• Has an init function that takes:

  • The appInfo
  • recipeList: RecipeModule[]

• Provides an abstract recipeModule class that modules must extend:

  • Provides methods to asks a module if they can handle the current route (excluding websiteBasePath).
  • Get and manage the module's rId. The rId will be an argument to the constructor.
  • Exposes a function that modules must implement to handle the current route.

• Recipe modules must be build such that parent recipe's rId can be propagated to them. This is how

You need to pass this along with whatever else the user has give. So the header should look like (Or something equivalent):

let userHeaders = {};
if (config.refreshAPICustomHeaders !== undefined) {
   usersHeaders = config.refreshAPICustomHeaders
}
supertokensRequest.init({
   refreshAPICustomHeaders: {
      rid: this.getRecipeId(),
      ...usersHeaders
   }
})

We should allow the user to do the following in the render function:

• Check if a user is logged in or not
• Get the userId
• Get JWT payload

It's not recommended to do those in the render function at the moment because they all read the browser storage, which is slow. Since render can be called many times, that means each time a slow operation like the above will cause a UI lag.

Issues from past PR that are still there:

• Are the issues related to testing solved?
  • lots of the tests threw CONNECTION REFUSED
• Is the function SuperTokensRouteWithRecipeId in superTokensRoute.tsx equal to the render function of that component?

New issues / questions

• In index.ts of the recipe, you also need to export all the functions inside the Wrapper class, so that they can be accessed via import * as ___ from "..."
• In index.ts of the recipe, you also need to export all the things that are outside the Wrapper class so that they can be accssed via import ___ from "..."
• Please check your code for if statements without {}. I found one instance in getPathsToComponentWithRecipeIdMap in superTokens.tsx. Also, please use explicit comparison.. Does if ([]) equal to if (false)?
• This is a small issue, but there are some cases in which you call a static function from a non static function. The static function then gets the instance and calls the relevant non static function. Example is in getMatchingComponentForRouteAndRecipeId in superTokens.tsx
• getMatchingComponentForRouteAndRecipeId should take path as NormalisedURLPath. Not as a string. You want to convert this class to a string as late as possible, and only if absolutely necessary.
• getMatchingComponentForRouteAndRecipeId is written in a way that if a rId is provided in the URL, but that doesn't match any recipe, then we will treat it as if rId is not there is not there in the URL. Is this expected?
• In canHandleRoute, I think it's better to call the getRoutingComponent and check if that is undefined or not. This way, it's guaranteed that if we change the routing algo, then canHandleRoute will change accordingly.
• We should abstract away new NormalisedURLPath(window.location.pathname) into a function in utils. This way, if it turns out later that window.location.pathname is not correct, then we can easily change it. It also gives the advantage that we do not use window.location.pathname without first normalising it anywhere.
• In httpRequest.ts, if config is undefined, and we do ...config, will that throw an error?
• In httpRequest.ts, no need for supertokens-auth-react-version
• Can we move some of the constants, styles and components specific to recipes into their folder?
• Do we need canHandleRoute and getRoutingComponent in recipeModule.ts?
• If we are allowing users to change the background for the default routes, then where will that go? If we allow them to do it via style in the init, what if they give a different colour for sign up and sign in?
• For handling signInAndUpFeature object in the init call, I suggest you create a function like validateAndNormaliseSignInAndUpFeatureConfig which takes what the user gave and returns the fully normalised config back which you can then propagate to the features. So this function will:
  • normalise all the form fields, and insert default ones if necessary
  • merge the styles?
  • Set onSuccessRedirectURL to / if the user has not provided one
  • etc..
• For handling disabledDefaultImplementation, you are using the user's provided config before actually normalising it first. The first thing we want to do in the recipe is normalise all of the user's input, and add all defaults and then use that object.
• Can we convert SignInAndUp to a normal class that extends React? Having functions inside a function, not being able to control things like shouldComponentUpdate etc.. are all restrictive. I prefer to use functional component only if they are not complex.
• validateFormOrThrow in utils.ts does not throw any error.
• Like all other modules, even for SignInAndUp, I prefer if we first normalise (add defaults) the config / props given by the user and store the normalised version of that as an instance variable. So this way, when you are calling the signUp API, you do not have to check if props.signUpApi === undefined. You can just do this.config.signUpApi(..).
• In the components folder inside emailpassword, we should have a themeComponent folder
• signUpApi and signInApi in emailPassword.ts do not take care of error handling?
• Error handling for API calls needs to be done assuming the user can use axios or fetch.
• In SignInAndUp, signInAPI should not change state.. it should only return the response of the API call in a normalised way. We need to segregate API call layer from state layer of a component.
• Use useEffect carefully
• The API call functions take and return a JSON. If the status is >= 300, they throw an error. So we don't really care if the user uses axios or fetch. However, we have to follow this same semantic in our default implementations
• Need to figure out if the SuperTokensRouteWithRecipeId runs every render or every route change?
• Add the fdi-version header to requests as per supertokens/frontend-driver-interface#2.

Related to previous code review (#20)

• The index.ts of emailpassword does not does not export the components as part of the default export. Is that intentional? This means that users cannot do <EmailPassword.SignInAndUp />

• Why do you have export { SessionAPIWrapper }; in the index.ts of session recipe?

• getMatchingComponentForRouteAndRecipeId should take path as NormalisedURLPath. Not as a string. You want to convert this class to a string as late as possible, and only if absolutely necessary.

• The below

  (My comment) getMatchingComponentForRouteAndRecipeId is written in a way that if a rId is provided in the URL, but that doesn't match any recipe, then we will treat it as if rId is not there is not there in the URL. Is this expected?

  (Your reply) "Yes that's what we decided in our previous discussion around routing. If no rid is provided, or if rid is unknown we want to use the first recipe feature that matches."

  Are you sure we had this conversation? Because on the backend, if the rId is there, but it not matching any route, then I pass the request to the user.

• If we are allowing users to change the background for the default routes, then where will that go? If we allow them to do it via style in the init, what if they give a different colour for sign up and sign in?

• The below

  validateFormOrThrow in utils.ts does not throw any error.

  It actually does throw if length is different as per our conversation.

  Yes, but the name suggests that if the form is valid, then it will not throw an error, else it will. Which means, as a consumer of this function I would not care about the return type.

• It would be nice to move the API calls in SignInAndUp (and other components) to a different file that only contains API calls.

• Does fetch throw an error in case the status code is >= 300?

New issues

• onSuccessRedirectURL in the config can be a full URL right (in case of multi-tenancy)? In that case, you do not want to normalise it with NormalisedURLPath. Perhaps there is no normalisation for this value that is required? (This applies to signInUp and to resetPassword feature)

• When creating the defaultSignInFields in utils.ts (line 146), I think you forgot to add the email validator? If not that, you added the default password validator, overriding the user's password validator?

• I feel the way the signIn form fields has been made can have bugs cause it's hard to understand for the following reasons (feel free to ignore this if you are very confident that your code regarding this is bug free):

  • Using reduce
  • Doing reduce on signup form fields, then doing a reduce on user's input form fields, then merging the two. Can this be made simpler somehow?

• You are doing an implicit check with if (config.resetPasswordURL) { in utils.ts. What is resetPasswordURL === ""? Then is if ("") === if (true). Please, no more implicit checks. Which also means no more of config.style || {}, and no more if (this.props.onHandleForgotPasswordClicked) etc..

• I may be wrong, but the normaliseResetPasswordUsingTokenFeature does not seem to take the email and password validators from sign up.

• There are two places you use Object.assign, but everywhere else you use the spread operator. I prefer you stick on one style of coding.

• I am not 100% sure about this, but extending component classes with PureComponent may be better than doing so with Component. PureComponent does a shallow comparison of state and props to check if a render is needed which may be more efficient than Component which always returns true by default.

• When setting state in components that are class based, you should use this.setState(oldState => {...}) method. This is a more reliable method since you are guaranteed to mutate the "actual current state" in case the setState runs after a couple other setStates.

• The return type of signUpAPI (line 197 in SignInAndUp) (and other similar functions) is very confusing. Please rename response and responseJson to something better.

• The FormBase component is not using controlled input. I'm not sure what the benefit of that is, but maybe it's worth doing that?

• The state for SignInAndUp can be improved from a type point of view (Not sure if the below will exactly work according to your logic, but something like this where you divide the state into clear distinct sub types based on the state of the form):

  {
     status: "LOADING"
  } | {
     status: "NOT_SUBMITTED",
  } | {
     status: "SUBMITTED",
     user: User,
     responseJson: any
  }
  

  • From the types, we know that if the status is LOADING, then no other state matters. Your render function will be forced to check the status first and render correctly = no bugs
  • If the form has not been submitted yet, then there will be no user and responseJson object. Again, your functions will be forced to make this check = no bugs.
  • If the form has been submitted, then there MUST be a user and responseJson object.
  • Try doing something similar for all the components if possible. It makes everything more clear.

• Since supertokens-website is in the dependencies in package.json, you can use a normal import statement for it in session.ts

• addAxiosInterceptors, setAuth0API and getAuth0API are missing from the session recipe.

• Is all the UI responsive for tablet, mobile, 4k screens etc..?

Please make sure this doesn't happen with any other navgiation as well.

• tests/end-to-end/signin.test.js
• tests/end-to-end/signup.test.js

This issue is intended as a tracker on which versions we will support for our peer dependencies.

• react: Currently ^16.14.0 but we can definitely go down as this is the latest version that was installed when I ran npm install react

• react-router-dom: Currently ^5.2.0 which is latest. I think we could support up to 4.0 since the API never changes. I'll have to test.

Field decorators

We want to allow to extend the default EmailPassword theme to add custom behaviour such as :

• Password strength meters
  • weak | good | excellent indicators
  • password time to crack
  • Password is in top 100/1k most used password
• Email suggestions: "We noticed that you are using a Gmail account, do you have a work email?"

We want to do this without having to:

• Implement a new theme that behaves exactly the same as the default except for that one small behaviour.
• Ask customers to disable default implementation to overwrite the default theme.

Solution

The idea is to add an optional property called decorators to formFields. This decorator consists of a React element that will be inserted below the input field and that will take its value as props.

Example:

SuperTokens.init({
  appInfo: {},
  recipeList: [
    EmailPassword.init({
      signInAndUpFeature: {
        onSuccessRedirectURL: '/dashboard',
        signUpForm: {
          formFields: [{
              id: "password",
              decorator: <PasswordStrengthMeterDecorator />
            }]
         }
       }
    })
   ] 
});

With PasswordStrengthMeterDecorator a React element that takes a value as props (the pasword which is updated on each changes), and displays a UI below the input.
We can provide few decorators as examples but the idea is that the developers can implement their own to extend the default theme easily if they like.

Implementation

Implementation is straightforward, we simply need to:

• Add decorator?: JSX.Element to FormFieldBaseConfig type.
• Prevent propagating SignUp decorators to SignIn
• Add onChange in input.tsx` to make sure that the state is updated each time the value is updated (as opposed to using React refs).
• Insert the decorator element, give it input value as props.

Autocomplete for extensions such as 1Password, Dashlane and Lastpass does not work within shadow dom:

Without shadow dom:

With shadow dom

It only works when shadow dom is disabled. This is not an issue related to us directly as stated in https://1password.community/discussion/79137/shadow-dom-polymer-forms-do-not-fill-companion-extension-v4-7-4-now-supports-shadow-doms

Basically 1Password does a querySelectorAll which won't work with shadow Dom. That is their responsibility to catch shadow root dom component in the future with their extensions. It seems pretty straightforward and I don't understand why they haven't done such a thing yet (maybe not a top priority for them).

I think it's pretty important for us as an open source authentication solution to support password managers.
We can't provide support for password managers while shadow root is enabled at the moment. Password manager providers do not seem to be actively working on solving the issue.

Bunch of other open source frameworks are facing this issue: Ionic, Polymer.
Other resource on the subject: WICG/webcomponents#572

Solution proposed

In case they want support for password managers, we could provide an "obscure" config to allow users to disable shadow dom letting them know that doing such a thing might result in CSS clashes because they are removing CSS encapsulation.

SuperTokens.init({
  appName: "",
  {...},
  useShadowDom: false // default true

• They should care about Supertokens components styling mainly as we are using a solution that makes sure there is no clashes with their CSS EVEN IF shadow root is disabled.
• Only CSS that we are loading and that would be loaded on their website is the fontFamily Rubik and it should not impact their website but they should make sure.

• async method that calls the FDI corresponding API /auth/signout .
• No init configs for this feature.
• No UI required.

To use like this:

import { signOut } from "supertokens-auth-react/recipe/emailpassword"

function NavBar () {
  function async onLogout () {
      await signOut();
      window.location.href = "/";
  }
  return (
    <ul>
         <li>Home</li>
         <li onClick={onLogout}>Logout</li>
    </ul>
  )
}

You can do so by copying the tests on the supertokens-website. You need to run tests for axios and for fetch. Just adding one test for both should be enough since this SDK is a wrapper to the actual SDK. The one test can be a simple refresh test as done here:

• For axios: https://github.com/supertokens/supertokens-website/blob/master/test/axios.test.js#L261
• For fetch: https://github.com/supertokens/supertokens-website/blob/master/test/fetch.test.js#L223

• Getting a session expired status code from sign out should not throw that error to the user. Instead, it should be treated as successful. Please make sure to not hardcode 401 and instead use what the user has given to the session recipe's expired status code.

• You are giving the header as rid: EmailPassword.RECIPE_ID. Arent you supposed to instead do rid: this.getRecipeId() so that if recipe is being overided, the new recipeId will be used? Please make sure this type of error does not exist anywhere else.

• Since we are in this subject, why do you make the react components propogate the rid to the functions in the EmailPassword recipe? Instead, those functions should add the rid header like rid: this.getRecipeId(). Which is much cleaner.

This is so that the end user doesn't see messages for password syntax validation during sign in (which no site does)

• Instead, return WRONG_CREDENTIALS_ERROR
• The password is not optional. So it cannot be "". If the password is "", then it should be a form_field_error
• If the email and the password are invalid, then it should show an email form_field_error only.

Similar to https://login.retool.com/auth/login for incorrect email.

Implement animations for default themes:

• formFields error
• general Error

Steps to reproduce:
1/ Go to /auth/reset-password
2/ Component is not displayed.

We would like to call this function at the top of the file near the import statement. However, that means this may get executed before the supertokens.init function. In this case, it throws Error: No instance of Function found. Make sure to call the "init" method. error. A few points:

• "No instance of Function found" is a bad error message
• Make sure to call the "init" method. -> Make sure to call the "supertokens.init" method.
• The error happens because in addAxiosInterceptors, we call the getInstanceOrThrowError function. Because the init function has not been called yet, an error is thrown. We can fix this by simply calling the supertokens-website functions from the static functions directly (for all the functions related to session).

Emotion v11 and shadowDom support for v11

React allows to add styles directly to its components using the style props.
Nevertheless, some styling libraries are becoming very popular to do styling in react. Styled components and aphrodite coming first.

• Pros of using React:
  • no need for external dependencies.

Pros of using an external lib:

• Support for mouse events (:hover, :visited) and others (:after :before)
• Support for media query resizing ("@media (max-width: 440px)")
• Use classes (semi randomised) instead of polluting the dom with style attributes (inspecting the code looks much nicer
• Integrated merging of styles (default + user provided styles)

Cons:

• lightweight library but still a dependency (but it helps us to get rid of another dependency => shadow root)

I think we should go with Aphrodite because after exploring react styling for an hour or so, I realised that it is too restrictive and we would need to add a stylesheet at some point (for media query particularly).

Shadow root does not prevent from propagating the CSS from parent to child, it only works the other way around, making sure that 3rd party styling does not clash with parent styling. We don't have any good way to prevent parent style to clash with our modules. Shadow-root helps to isolate an inserted stylesheet but if we only use randomised classes with aphrodite we do not need to isolate our component with a shadow root.

Is your code architected in a way to allow:

• Yours to have multiple sign up pages?
  • One default, rest custom
  • All custom
• Same as above, but for sign in

• In package.json
  Replace

 "supertokens-website": "git+https://github.com/supertokens/supertokens-website.git#5.0" 

• In test/server/package.json
  Replace

"supertokens-node": "git+https://github.com/supertokens/supertokens-node.git#3.0"

with npm package.

• import { SignInAndUpTheme } from "supertokens-auth-react/recipe/emailpassword"
• There should be two sub themes in here: SignUpTheme and SignInTheme.
• Themes should use a ThemeProvider which can be used to overwrite the default theme based on the passed props.

• import { SignInAndUp } from "supertokens-auth-react/recipe/emailpassword"
• The default implementation should be in ${websiteBasePath}/
• The default theme should be SignInAndUpTheme
• Props for this component (all optional, all must have a default implementation):
• Will pass the following down to the Theme component (along with whatever the user passes):
  • signUpForm Object
  • signInForm Object
• They must be responsible to render a shadow-root to prevent CSS clashes
• Should have its own config in the init function as explained in this comment
• On sign in / up, this feature will first apply the frontend validators. If they pass, it will call the API. This means that the user does not need to call the validators themselves in case they are building thier own theme

It's not about the number of steps required, but about making sure that each step runs successfully and at the end, I am able to run the tests.

This happens when I use manual routing only.

• When clicking on "Forgot password", it still shows the sign in form, even though the path is /auth/reset-password.
• When the path is changed to /auth/reset-password, it should also contain the rid. In general, whenever we navgiate to a page controlled by us (from a button controlled by us), we want to add the rid. For example, in case of forgot password, we want to add the rid to the route if they have not provided the resetPasswordURL in the signInForm object during init.

By default the "Sign In" page is displayed.

Should we:

• keep it as it is (login first which is what most people do)
• default to sign up (which is good to increase conversion for start ups)
• allow the user to decide by adding a config:

EmailPassword.init({
   signInAndUpFeature: {
      defaultToSignIn: true
   }
})

Another thing to take into account is:
People might want to post/tweet/email links to their signup page, so if we keep the "Sign-In" page, we should have a way to add a query param to toggle to Sign-up.

Ex https://app.circleci.com/pipelines/github/supertokens/supertokens-auth-react/16/workflows/12173e17-3328-4dcc-aea1-4ea8461ce888/jobs/11

All routing functions / components

With React route

import {SuperTokensRoute} from "supertokens-auth-react"

<Router history={history}>
   <NavBar />
      <Switch>
         <SuperTokensRoute />
      </Switch>
   <Footer />
</Router>

Reach Router ?

• [] TODO @NkxxkN make researches, decide wether we should support reach router on not. https://reach.tech/router/

Manual routing

import SuperTokens from "supertokens-auth-react"

if (SuperTokens.canHandleRoute()) {
   return SuperTokens.getRoutingComponent()
}

• Internally SuperTokensRoute uses SuperTokens.getRoutingComponent()

"You will receive a password recovery link at your email address in a few minutes" -> Saying few minutes is not a good idea. Change it to "Please check your email for the password recovery link"