Skip to content
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

[RFR] [BC break] Reimplement auth logic using hooks #3655

Merged
merged 26 commits into from
Sep 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
878437d
Reimplement auth logic using hooks
fzaninotto Sep 5, 2019
483a501
Publish one hook for each verb
fzaninotto Sep 6, 2019
67b2227
Better typing
fzaninotto Sep 6, 2019
d0b47bb
Fix unit tests
fzaninotto Sep 6, 2019
eea7b83
remove useless memoization
fzaninotto Sep 6, 2019
5080728
Fix infinite loop
fzaninotto Sep 6, 2019
ee797b5
Fix linter warning
fzaninotto Sep 6, 2019
456bfea
Fix unit tests
fzaninotto Sep 6, 2019
15004f3
Fix performance penalty of useSelector
fzaninotto Sep 6, 2019
6966b92
don't use old auth actions in CoreadminRouter
fzaninotto Sep 6, 2019
3f557b8
Remove useless reducer
fzaninotto Sep 6, 2019
2f52457
use useLogin in demo example
fzaninotto Sep 6, 2019
ac9c5be
loading reducer no longer watches user checks
fzaninotto Sep 6, 2019
392379d
Show initial loader only if getPermissions takes more than one second
fzaninotto Sep 7, 2019
d0442c8
add useLogoutIfAccessDenied hook
fzaninotto Sep 8, 2019
55f2599
Use the new hook in dataProvider
fzaninotto Sep 8, 2019
b57de41
Test return value of useLogoutIfAccessDenied
fzaninotto Sep 8, 2019
a40b274
Update authentication documentation
fzaninotto Sep 8, 2019
9bb4e64
fix typo in Authorization documentation
fzaninotto Sep 8, 2019
f7b4518
Fix unit tests
fzaninotto Sep 8, 2019
94dfbe4
Fix e2e test
fzaninotto Sep 9, 2019
9730e6c
[BC Break] Do not pass location to AUTH_GET_PERMISSIONS call
fzaninotto Sep 9, 2019
49abe21
Allow authProvider to override redirection url in AUTH_CHECK and AUTH…
fzaninotto Sep 9, 2019
080de6a
Redirect to homepage when visiting the login page while connected
fzaninotto Sep 9, 2019
cdd94ea
Mention no more auth actions in upgrade guide
fzaninotto Sep 9, 2019
d079b11
Fix e2e tests
fzaninotto Sep 9, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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