Add possibility for custom role handling

[StuckiSimon]

While most use cases can be resolved using the <Authorize /> component there are some special ones which might require a custom handling.

Example:

• Redirect to route based on rights

  if (hasRole('admin')) {
  redirect('/admin')
  } else if (hasRole('accountant') {
  redirect('/booking')
  }

It is possible to implement these use cases with the existing component, but it seems odd to use <Authorize neededRoles=[] /> in such a case.

My proposal is a HOC (Higher Order Component):

Usage:

withRoles(Component)

This HOC could then also be used by the <Authorize /> component itself.

[sir-marc]

This could be done using the current !lacksRole('admin') but i agree, there could be a better api for this. What about an additional helper which does just that but is called hasRole?

I see that using that a HOC for cases where you have such custom rules all over the code base separate from each over would be better. But in those rare cases the user could create a HOC himself using our renderProp Component internally with the proposed api.

[StuckiSimon]

A render prop is also an absolutely valid solution. Serves the same purpose.

[sir-marc]

17 Resolves this request

[StuckiSimon]

As discussed in private chat

[sir-marc]

At the moment it is possible to create the desired behavior using: <Authorize neededRoles={[]}> ({ hasRole }) => { if (hasRole(role) redirect('/somepage'); } </Authorize>

I agree, passing an empty, useless array is cumbersome. So there I see two primary options: a) Adding an addition Component which does not require neededRoles and just passes down the hasRole and lacksRole helpers.

But i don't like to add one more Component to the API which does nothing special expect not requiring a neededRoles array.

b) Make the neededRoles prop on Authorize optional: This would just fulfill the usecase but it does also pass down the isAuthorized Prop which in that case would be useless. We could omit this prop, if no neededRoles is passed to the Component, but in that case the isAuthorized renderProp would be undefined (falsy) which might lead to unexpected behavior. One might think that isAuthorized must be true, because no roles are needed.

So we have the two options: pass down a pretty useless renderProp (in the documentation the behavior in such cases must be extensively documented) or omitting it and make room for confusion.

One third option might be to add a getter method to the object containing the renderProps (isAuthorized, etc) and throw an Invariant when the user tries to access isAuthorized when no neededRoles are present. The simplified Authorize Component would then look something like this:

const Authorize = (props) => {
  const renderProps = { 
    hasRole: role => true,
    get isAuthorized() {
      if (props.neededRoles) return this.isAuthorized();
      throw new Error('You should not access `isAuthorized` when neededRoles is not passed as props to the <Authorize /> Component')
    }
  };
  return props.children(renderProps)
}

While this usage would be fine:

<Authorize>
  {({ hasRole }) => {
    if (hasRole('admin')) return 'hi admin';
      return 'you are no admin';
  }}
</Authorize>

This would throw an error:

<Authorize>
  {({ hasRole, isAuthorized }) => {
    if (hasRole('admin')) return 'hi admin';
    return 'i dont even use isAuthorized';
  }}
</Authorize>

What do you think about this solution?

[StuckiSimon]

Your point of not increasing complexity for the consumer is valid. If they'd provide the same result and therefore cover the same use case - I'd agree that they must be the same component. But I'm not convinced trying to expose the minimal surface area is the best approach for designing a library. Merging different use cases into one component tries making different requirements the same. It's a step away from modularity and a step towards monolithic design. In my opinion react-authorizer should be open for extension - closed for modification.

By just providing <Authorize /> the library assumes it is aware of all possible use cases. Essentially meaning, that as soon as a consumer has a different use case he's got no way of extending the library in a manner which suits his requirements.

Proposal

Therefore I'd still suggest adding a new independent base component.

Using different components for different requirements is a rather common pattern in React (e.g. react-select).

Having specific components will ensure the intent of the specific action is clear. While it is important for a beginner to get started as quick as possible - it's also important advanced use cases can be incorporated using react-authorizer.

Example usages would look something like:

withRoles(({roles}) => ())

<Authorize requiredRoles={roles}>
  {({ isAuthorized, missingRoles, hasRole, lacksRole }) => ()}
</Authorize>

<RoleSwitch>
  {({ hasRole, lacksRole }) => {
    switch (true) {
      case hasRole('admin'): redirect('/admin') return;
      case hasRole('user'): redirect('/dashboard') return;
    }
  }}
</RoleSwitch>

Using this approach additional custom use cases can be implemented without changing the core of react-authorizer.

Notice: The naming is not a proposal - it's just a demonstration of the different use cases. I do not think all of them should be implemented.

Summary

In my opinion it's a question about the core design of react-authorizer: opinionated & monolithic vs. open-minded & modular. Of course both approaches have their advantages and disadvantages.

[StuckiSimon]

I've thought about the contract for a while. I think it would make sense to stick with two components:

1. AuthProvider
2. AuthConsumer (render prop) or withAuth() (HOC)

Regarding the render prop vs HOC debate for the Consumer: I'd go for the HOC, because it's enables you to use roles in all lifecycle methods out-of-the-box.

The Provider consumes roles and the Consumer provides roles - but that's really the only thing they do.

Then the user can import all the helpers required under the given circumstances. Such helpers are:

• hasRole
• lacksRole
• hasAllRoles
• lacksAnyRole
• getMissingRoles

Why not just provide these helpers as properties on roles? (roles.hasAll()) When a project requires custom helpers they should have a similar contract to the built-in ones.

A potential usage might look like:

import { withRoles, hasAllRoles } from '@kreativwebdesign/react-authorizer'
import { hasFullUserBasedAccess } from './custom-authorizer-helpers'
const OnlyVisibleForAccountantAdmin = withRoles(({ roles }) => {
  // use built-in helpers directly
  const isAuthorized = hasAllRoles(roles, ['admin', 'accountant'])
  // or create custom helper (which uses the built-in helpers under the hood)
  const isFullUser = hasFullUserBasedAccess(roles)
  // ...
})

It encourages projects to extract auth checks into helpers.

This approach provides a clear and simple API while being really flexible and open for extension.