Skip to content

Commit

Permalink
Merge pull request #3655 from marmelab/auth-side-effects
Browse files Browse the repository at this point in the history
[RFR] [BC break] Reimplement auth logic using hooks
  • Loading branch information
djhi authored Sep 10, 2019
2 parents eefc2f2 + d079b11 commit 47c7171
Show file tree
Hide file tree
Showing 46 changed files with 1,213 additions and 611 deletions.
55 changes: 54 additions & 1 deletion UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,13 @@ When you provide an `authProvider` to the `<Admin>` component, react-admin creat

If you didn't access the `authProvider` context manually, you have nothing to change. All react-admin components have been updated to use the new context API.

Note that direct access to the `authProvider` from the context is discouraged (and not documented). If you need to interact with the `authProvider`, use the new `useAuth()` and `usePermissions()` hooks, or the auth-related action creators (`userLogin`, `userLogout`, `userCheck`).
Note that direct access to the `authProvider` from the context is discouraged (and not documented). If you need to interact with the `authProvider`, use the new auth hooks:

- `useLogin`
- `useLogout`
- `useAuthenticated`
- `useAuthState`
- `usePermissions`

## `authProvider` No Longer Receives `match` in Params

Expand Down Expand Up @@ -959,3 +965,50 @@ const ExportButton = ({ sort, filter, maxResults = 1000, resource }) => {
);
};
```

## The `authProvider` no longer receives default parameters

When calling the `authProvider` for permissions (with the `AUTH_GET_PERMISSIONS` verb), react-admin used to include the pathname as second parameter. That allowed you to return different permissions based on the page. In a similar fashion, for the `AUTH_CHECK` call, the `params` argument contained the `resource` name, allowing different checks for different resources.

We believe that authentication and permissions should not vary depending on where you are in the application ; it's up to components to decide to do something or not depending on permissions. So we've removed the default parameters from all the `authProvider` calls.

If you want to keep location-dependent authentication or permissions logic, read the current location from the `window` object direclty in your `authProvider`, using `window.location.hash` (if you use a hash router), or using `window.location.pathname` (if you use a browser router):

```diff
// in myauthProvider.js
import { AUTH_LOGIN, AUTH_LOGOUT, AUTH_ERROR, AUTH_GET_PERMISSIONS } from 'react-admin';
import decodeJwt from 'jwt-decode';

export default (type, params) => {
if (type === AUTH_CHECK) {
- const { resource } = params;
+ const resource = window.location.hash.substring(2, window.location.hash.indexOf('/', 2))
// resource-dependent logic follows
}
if (type === AUTH_GET_PERMISSIONS) {
- const { pathname } = params;
+ const pathname = window.location.hash;
// pathname-dependent logic follows
// ...
}
return Promise.reject('Unknown method');
};
```

## No more Redux actions for authentication

React-admin now uses hooks instead of sagas to handle authentication and authorization. That means that react-admin no longer dispatches the following actions:

- `USER_LOGIN`
- `USER_LOGIN_LOADING`
- `USER_LOGIN_FAILURE`
- `USER_LOGIN_SUCCESS`
- `USER_CHECK`
- `USER_CHECK_SUCCESS`
- `USER_LOGOUT`

If you have custom Login or Logout buttons that dispatch these actions, they will still work, but you are encouraged to migrate to the hook equivalents (`useLogin` and `useLogout`).

If you had custom reducer or sagas based on these actions, they will no longer work. You will have to reimplement that custom logic using the new authentication hooks.

**Tip**: If you need to clear the Redux state, you can dispatch the `CLEAR_STATE` action.
8 changes: 5 additions & 3 deletions cypress/integration/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,17 @@ describe('Authentication', () => {
ListPage.navigate();
ListPage.logout();
ListPage.navigate();
cy.url().then(url => expect(url).to.contain('/#/login'));
cy.url().should('contain', '/#/login');
});
it('should not login with incorrect credentials', () => {
LoginPage.navigate();
ListPage.navigate();
ListPage.logout();
LoginPage.login('foo', 'bar');
cy.contains('Authentication failed, please retry');
});
it('should login with correct credentials', () => {
LoginPage.navigate();
ListPage.navigate();
ListPage.logout();
LoginPage.login('login', 'password');
cy.url().then(url => expect(url).to.contain('/#/posts'));
});
Expand Down
12 changes: 9 additions & 3 deletions cypress/integration/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ describe('Create Page', () => {
const LoginPage = loginPageFactory('/#/login');

beforeEach(() => {
LoginPage.navigate();
LoginPage.login('admin', 'password');
CreatePage.navigate();
CreatePage.waitUntilVisible();
});

it('should show the correct title in the appBar', () => {
Expand All @@ -40,6 +39,9 @@ describe('Create Page', () => {
});

it('should have a working array input with references', () => {
CreatePage.logout();
LoginPage.login('admin', 'password');
CreatePage.waitUntilVisible();
cy.get(CreatePage.elements.addAuthor).click();
cy.get(CreatePage.elements.input('authors[0].user_id')).should(
el => expect(el).to.exist
Expand All @@ -50,6 +52,9 @@ describe('Create Page', () => {
});

it('should have a working array input with a scoped FormDataConsumer', () => {
CreatePage.logout();
LoginPage.login('admin', 'password');
CreatePage.waitUntilVisible();
cy.get(CreatePage.elements.addAuthor).click();
CreatePage.setValues([
{
Expand Down Expand Up @@ -215,8 +220,9 @@ describe('Create Page', () => {
});

it('should not reset the form value when switching tabs', () => {
LoginPage.navigate();
CreatePage.logout();
LoginPage.login('admin', 'password');
CreatePage.waitUntilVisible();
UserCreatePage.navigate();

CreatePage.setValues([
Expand Down
6 changes: 3 additions & 3 deletions cypress/integration/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('List Page', () => {
});

it('should keep filters when navigating away and going back on given page', () => {
LoginPage.navigate();
ListPagePosts.logout();
LoginPage.login('admin', 'password');
ListPagePosts.setFilterValue('q', 'quis culpa impedit');
cy.contains('1-1 of 1');
Expand All @@ -96,7 +96,7 @@ describe('List Page', () => {
});

it('should allow to disable alwaysOn filters with default value', () => {
LoginPage.navigate();
ListPagePosts.logout();
LoginPage.login('admin', 'password');
ListPageUsers.navigate();
cy.contains('1-2 of 2');
Expand Down Expand Up @@ -181,7 +181,7 @@ describe('List Page', () => {
});

it('should accept a function returning a promise', () => {
LoginPage.navigate();
ListPagePosts.logout();
LoginPage.login('user', 'password');
ListPageUsers.navigate();
cy.contains('Annamarie Mayer')
Expand Down
19 changes: 12 additions & 7 deletions cypress/integration/permissions.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,22 @@ describe('Permissions', () => {
const EditPage = editPageFactory('/#/users/1');
const ListPage = listPageFactory('/#/users');
const LoginPage = loginPageFactory('/#/login');
const ShowPage = showPageFactory('/#/users/1/show', 'name');
const ShowPage = showPageFactory('/#/posts/1/show', 'title');
const UserShowPage = showPageFactory('/#/users/1/show', 'name');

describe('Resources', () => {
it('hides protected resources depending on permissions', () => {
LoginPage.navigate();
ShowPage.navigate();
ShowPage.logout();
LoginPage.login('login', 'password');
cy.contains('Posts');
cy.contains('Comments');
cy.contains('Users').should(el => expect(el).to.not.exist);
});

it('shows protected resources depending on permissions', () => {
LoginPage.navigate();
ShowPage.navigate();
ShowPage.logout();
LoginPage.login('user', 'password');
cy.contains('Posts');
cy.contains('Comments');
Expand All @@ -31,9 +34,10 @@ describe('Permissions', () => {

describe('hides protected data depending on permissions', () => {
beforeEach(() => {
ShowPage.navigate();
ShowPage.logout();
LoginPage.navigate();
LoginPage.login('user', 'password');
cy.url().then(url => expect(url).to.contain('/#/posts'));
});

it('in List page with DataGrid', () => {
Expand Down Expand Up @@ -79,9 +83,10 @@ describe('Permissions', () => {

describe('shows protected data depending on permissions', () => {
beforeEach(() => {
ShowPage.navigate();
ShowPage.logout();
LoginPage.navigate();
LoginPage.login('admin', 'password');
cy.url().then(url => expect(url).to.contain('/#/posts'));
});

it('in List page with DataGrid', () => {
Expand Down Expand Up @@ -109,12 +114,12 @@ describe('Permissions', () => {
});

it('in Show page', () => {
ShowPage.navigate();
UserShowPage.navigate();
cy.contains('Id');
cy.contains('Name');
cy.contains('Summary');
cy.contains('Security');
ShowPage.gotoTab(2);
UserShowPage.gotoTab(2);
cy.contains('Role');
});

Expand Down
5 changes: 3 additions & 2 deletions cypress/integration/tabs-with-routing.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ describe('Tabs with routing', () => {
const LoginPage = loginPageFactory('#/login');

beforeEach(() => {
LoginPage.navigate();
ShowPage.navigate();
ShowPage.logout();
LoginPage.login('admin', 'password');
cy.url().then(url => expect(url).to.contain('#/posts'));
cy.url().then(url => expect(url).to.contain('#/users'));
});

describe('in TabbedLayout component', () => {
Expand Down
9 changes: 8 additions & 1 deletion cypress/support/CreatePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ export default url => ({
descInput: '.ql-editor',
tab: index => `.form-tab:nth-of-type(${index})`,
title: '#react-admin-title',
userMenu: 'button[title="Profile"]',
logout: '.logout',
},

navigate() {
cy.visit(url);
},

waitUntilVisible() {
cy.get(this.elements.submitButton);
cy.get(this.elements.submitButton).should('be.visible');
},

setInputValue(type, name, value, clearPreviousValue = true) {
Expand Down Expand Up @@ -89,4 +91,9 @@ export default url => ({
gotoTab(index) {
cy.get(this.elements.tab(index)).click();
},

logout() {
cy.get(this.elements.userMenu).click();
cy.get(this.elements.logout).click();
},
});
9 changes: 8 additions & 1 deletion cypress/support/ShowPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,24 @@ export default (url, initialField = 'title') => ({
snackbar: 'div[role="alertdialog"]',
tabs: `.show-tab`,
tab: index => `.show-tab:nth-of-type(${index})`,
userMenu: 'button[title="Profile"]',
logout: '.logout',
},

navigate() {
cy.visit(url);
},

waitUntilVisible() {
cy.get(this.elements.field(initialField));
cy.get(this.elements.field(initialField)).should('be.visible');
},

gotoTab(index) {
cy.get(this.elements.tab(index)).click();
},

logout() {
cy.get(this.elements.userMenu).click();
cy.get(this.elements.logout).click();
},
});
Loading

0 comments on commit 47c7171

Please sign in to comment.