AxaFrance / oidc-client

Light, Secure, Pure Javascript OIDC (Open ID Connect) Client. We provide also a REACT wrapper (compatible NextJS, etc.).
MIT License
597 stars 160 forks source link

Skip Discovery Phase and Define Identity Endpoints #763

Closed slaterx closed 2 years ago

slaterx commented 2 years ago

Issue and Steps to Reproduce

Hi team,

Great job creating this lib, it's really cool and makes it very easy to implement secure SPAs.

I have a quite hard requirement on my hand and I am seeking clarifications about the best way to implement it. I have an implementation of OIDC to make but the identity server cannot and will not add our domain to CORS. How can we skip the discovery phase and give the actual endpoints that should be called? This identity server supports authorization code and implicit, but I read here (https://github.com/AxaGuilDEv/react-oidc/issues/751) that implicit is not supported.

Hence the question, how can I skip discovery and define the Authorization, Token, UserInfo and other endpoints to avoid the CORS issue?

Versions

v5.4.2

Screenshots

Expected

const configuration = {
  client_id: 'interactive.public.short',
  redirect_uri: 'http://localhost:4200/authentication/callback',
  silent_redirect_uri: 'http://localhost:4200/authentication/silent-callback', // Optional activate silent-signin that use cookies between OIDC server and client javascript to restore the session
  scope: 'openid profile email api offline_access',
  authority: {
    authorization: 'https://demo.identityserver.io/authorization',
    token: 'https://demo.identityserver.io/token',
    userinfo: 'https://demo.identityserver.io/userinfo',
    introspect: 'https://demo.identityserver.io/introspect',
    jwks: 'https://demo.identityserver.io/jwks',
    issuer: 'https://demo.identityserver.io/issuer',
  }
  service_worker_relative_url:'/OidcServiceWorker.js',
  service_worker_only:true,
};

Actual

Only authority variable is documented.

Additional Details

guillaume-chervet commented 2 years ago

I @slaterx thank you for your issue. It is in the roadmap. If you need it now i can set it up Quickly.

Implicit is not supported by choice of enforcing security first.

slaterx commented 2 years ago

Hi @guillaume-chervet,

Thank you for your quick reply, that was fast!

Yeah, we are kinda in a hurry, so that would be appreciated. If you need a hand, just let me know how I can help you, happy to assist in a PR to get this feature done!

Reading your source, it seems that the easiest way is to add an openid AuthorizationServiceConfigurationJson prop to OidcConfiguration then add some conditionals on initAsync, is that right?

guillaume-chervet commented 2 years ago

Yeah exactly a condition in initAsync to always return the predefined configuration. I can make a pullrequest and a beta version for you in few hours.

guillaume-chervet commented 2 years ago

If you want you can try to make a pullrequest your self.

slaterx commented 2 years ago

I'll try my luck, let's see where it takes me 😁

slaterx commented 2 years ago

@guillaume-chervet this was the closest I could get, I got stuck here:

image

Hopefully this is a good start and I just missed a comma somewhere πŸ˜…

guillaume-chervet commented 2 years ago

oO

khyapate commented 2 years ago

We have similar issue as mentioned here, wondering are we going to fix this issue soon?

guillaume-chervet commented 2 years ago

Hi @khyapate , thank you for the information. I will fixed it as soon as i can. I hope i can fixe it before the end of the next monday πŸ˜€

guillaume-chervet commented 2 years ago

@slaterx and @khyapate , I have push yesterday a version with the working feature. Does it resolve your use case?

khyapate commented 2 years ago

Thank you so much @guillaume-chervet for your quick response. Yes, it resolved my use case. thanks !

slaterx commented 2 years ago

Hi @guillaume-chervet, thank you for taking over my PR and delivering it! Unfortunately, I did not have time to look at it again and try to get it fixed.

So, testing your release, I do not get the CORS issue any longer, but now service worker won't load and I get the following error:


OidcSecure.tsx:22 Uncaught TypeError: Cannot read properties of undefined (reading 'tokens')
    at OidcSecure (OidcSecure.tsx:22:1)
    at renderWithHooks (react-dom.development.js:16175:1)
    at mountIndeterminateComponent (react-dom.development.js:20913:1)
    at beginWork (react-dom.development.js:22416:1)
    at HTMLUnknownElement.callCallback (react-dom.development.js:4161:1)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:4210:1)
    at invokeGuardedCallback (react-dom.development.js:4274:1)
    at beginWork$1 (react-dom.development.js:27405:1)
    at performUnitOfWork (react-dom.development.js:26513:1)
    at workLoopSync (react-dom.development.js:26422:1)
OidcSecure  @   OidcSecure.tsx:22
renderWithHooks @   react-dom.development.js:16175
mountIndeterminateComponent @   react-dom.development.js:20913
beginWork   @   react-dom.development.js:22416
callCallback    @   react-dom.development.js:4161
invokeGuardedCallbackDev    @   react-dom.development.js:4210
invokeGuardedCallback   @   react-dom.development.js:4274
beginWork$1 @   react-dom.development.js:27405
performUnitOfWork   @   react-dom.development.js:26513
workLoopSync    @   react-dom.development.js:26422
renderRootSync  @   react-dom.development.js:26390
performSyncWorkOnRoot   @   react-dom.development.js:26041
flushSyncCallbacks  @   react-dom.development.js:12009
flushSyncCallbacksOnlyInLegacyMode  @   react-dom.development.js:11988
scheduleUpdateOnFiber   @   react-dom.development.js:25438
dispatchSetState    @   react-dom.development.js:17389
(anonymous) @   browser.js:1802
call    @   browser.js:1802
applyTx @   browser.js:1802
handlePop   @   browser.js:1802

I tried to run the local development without HTTPS (thinking it could be because of the lack of certificates), but that way, I get an error about hook calls:


react.development.js:207 Warning: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.
printWarning @ react.development.js:207
error @ react.development.js:181
resolveDispatcher @ react.development.js:1590
useState @ react.development.js:1619
OidcProvider @ OidcProvider.tsx:86
renderWithHooks @ react-dom.development.js:14985
mountIndeterminateComponent @ react-dom.development.js:17811
beginWork @ react-dom.development.js:19049
beginWork$1 @ react-dom.development.js:23940
performUnitOfWork @ react-dom.development.js:22776
workLoopSync @ react-dom.development.js:22707
renderRootSync @ react-dom.development.js:22670
performSyncWorkOnRoot @ react-dom.development.js:22293
scheduleUpdateOnFiber @ react-dom.development.js:21881
updateContainer @ react-dom.development.js:25482
(anonymous) @ react-dom.development.js:26021
unbatchedUpdates @ react-dom.development.js:22431
legacyRenderSubtreeIntoContainer @ react-dom.development.js:26020
render @ react-dom.development.js:26103
./src/index.tsx @ index.tsx:9
options.factory @ react refresh:6
__webpack_require__ @ bootstrap:24
(anonymous) @ startup:7
(anonymous) @ startup:7
react.development.js:207 Warning: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.
printWarning @ react.development.js:207
error @ react.development.js:181
resolveDispatcher @ react.development.js:1590
useState @ react.development.js:1619
OidcProvider @ OidcProvider.tsx:86
renderWithHooks @ react-dom.development.js:14985
mountIndeterminateComponent @ react-dom.development.js:17811
beginWork @ react-dom.development.js:19049
callCallback @ react-dom.development.js:3945
invokeGuardedCallbackDev @ react-dom.development.js:3994
invokeGuardedCallback @ react-dom.development.js:4056
beginWork$1 @ react-dom.development.js:23964
performUnitOfWork @ react-dom.development.js:22776
workLoopSync @ react-dom.development.js:22707
renderRootSync @ react-dom.development.js:22670
performSyncWorkOnRoot @ react-dom.development.js:22293
scheduleUpdateOnFiber @ react-dom.development.js:21881
updateContainer @ react-dom.development.js:25482
(anonymous) @ react-dom.development.js:26021
unbatchedUpdates @ react-dom.development.js:22431
legacyRenderSubtreeIntoContainer @ react-dom.development.js:26020
render @ react-dom.development.js:26103
./src/index.tsx @ index.tsx:9
options.factory @ react refresh:6
__webpack_require__ @ bootstrap:24
(anonymous) @ startup:7
(anonymous) @ startup:7
react.development.js:1620 Uncaught TypeError: Cannot read properties of null (reading 'useState')
    at useState (react.development.js:1620:1)
    at OidcProvider (OidcProvider.tsx:86:1)
    at renderWithHooks (react-dom.development.js:14985:1)
    at mountIndeterminateComponent (react-dom.development.js:17811:1)
    at beginWork (react-dom.development.js:19049:1)
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945:1)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994:1)
    at invokeGuardedCallback (react-dom.development.js:4056:1)
    at beginWork$1 (react-dom.development.js:23964:1)
    at performUnitOfWork (react-dom.development.js:22776:1)
useState @ react.development.js:1620
OidcProvider @ OidcProvider.tsx:86
renderWithHooks @ react-dom.development.js:14985
mountIndeterminateComponent @ react-dom.development.js:17811
beginWork @ react-dom.development.js:19049
callCallback @ react-dom.development.js:3945
invokeGuardedCallbackDev @ react-dom.development.js:3994
invokeGuardedCallback @ react-dom.development.js:4056
beginWork$1 @ react-dom.development.js:23964
performUnitOfWork @ react-dom.development.js:22776
workLoopSync @ react-dom.development.js:22707
renderRootSync @ react-dom.development.js:22670
performSyncWorkOnRoot @ react-dom.development.js:22293
scheduleUpdateOnFiber @ react-dom.development.js:21881
updateContainer @ react-dom.development.js:25482
(anonymous) @ react-dom.development.js:26021
unbatchedUpdates @ react-dom.development.js:22431
legacyRenderSubtreeIntoContainer @ react-dom.development.js:26020
render @ react-dom.development.js:26103
./src/index.tsx @ index.tsx:9
options.factory @ react refresh:6
__webpack_require__ @ bootstrap:24
(anonymous) @ startup:7
(anonymous) @ startup:7
react-dom.development.js:20085 The above error occurred in the <OidcProvider> component:

Does this mean we have now a minimum supported version of react/react-dom to use?

Thanks, Gleidson

guillaume-chervet commented 2 years ago

Hi @slaterx , are you using react dom? If no what is your use case? Do you have a sample of your code? The message explain 3 steps to check about hooks.

slaterx commented 2 years ago

@guillaume-chervet yes, we are using react-dom:


import React from 'react';
import ReactDOM from 'react-dom';
import './index.scss';
import App from './App';
import { store } from './app/store';
import { Provider } from 'react-redux';
import reportWebVitals from './reportWebVitals';

ReactDOM.render(
  <React.StrictMode>
    <Provider store={store}>
      <App />
    </Provider>
  </React.StrictMode>,
  document.getElementById('root')
);

// If you want to start measuring performance in your app, pass a function
// to log results (for example: reportWebVitals(console.log))
// or send to an analytics endpoint. Learn more: https://bit.ly/CRA-vitals
reportWebVitals();

And we are also using react-router-dom:


import React from 'react';
import {HashRouter, Route, Routes} from 'react-router-dom';
import {OidcProvider, withOidcSecure} from '@axa-fr/react-oidc-context';
import (...)

const configuration = {
    client_id: 'f00,
    redirect_uri: 'http://localhost:3000/authentication/callback',
    silent_redirect_uri: 'http://localhost:3000//authentication/silent-callback',
    scope: 'openid',
    authority: 'https://demo.duendesoftware.com',
    authority_configuration: {
        authorization_endpoint: "https://demo.duendesoftware.com/connect/authorize",
        end_session_endpoint: "https://demo.duendesoftware.com/connect/endsession",
        revocation_endpoint: "https://demo.duendesoftware.com/connect/revocation",
        token_endpoint: "https://demo.duendesoftware.com/connect/token",
        userinfo_endpoint: "https://demo.duendesoftware.com/connect/userinfo",
    },
    service_worker_relative_url:'/org/repo/OidcServiceWorker.js',
    service_worker_only: false,
    token_request_extras: {'client_secret': 'b4r$}
};

const configurationName = 'identity-server';

class App extends React.Component<any, any> {

  render() {
    return (
      <div className="App">
          <OidcProvider configuration={configuration} configurationName={configurationName}>
              <Header aria-label="header">
                <HeaderName href="#" prefix="">
                  My Header
                </HeaderName>
                <HeaderNavigation aria-label="Header">
                  <HeaderMenuItem href="/">All Cases</HeaderMenuItem>
                  <HeaderMenuItem href="#/add-case">New Case</HeaderMenuItem>
                  <HeaderMenuItem href="#/edit-case">Edit Case</HeaderMenuItem>
                </HeaderNavigation>
              </Header>
              <Content>
                <HashRouter>
                  <Routes>
                    <Route path="/" element={<AllCasesPage />} />
                    <Route path="/add-case" element={<NewCasesPage />} />
                    <Route path="/documentation" element={<DocumentationPage />} />
                    <Route path="/edit-case" element={<SecureEdit />} />
                    <Route path="/edit-secure" element={withOidcSecure(Edit)} />
                  </Routes>
                </HashRouter>
              </Content>
              <Footer />
          </OidcProvider>
      </div>
    )
  }
}

export default App;
guillaume-chervet commented 2 years ago

I think i understood. What happen if you switch on the new package @axa-fr/react-oidc? It is just Γ  rename.

It may work

slaterx commented 2 years ago

Updating the name made the hook issues go away, but the app is now stuck on loading:

image

I believe that there's something in the oidc code which puts the page into a wait mode (loading) while we do the discovery (now disabled because we have the auth endpoints). But I can't pinpoint myself where in the code that is - would that piece of code in the screenshot show up to you where that loading step might be? We will need to also add the conditional there, to move along since we won't make the discovery call when the endpoints are defined already.

guillaume-chervet commented 2 years ago

Thank you for the information. I pretty think it is a bug. i'am trying to reproduce it.

khyapate commented 2 years ago

@guillaume-chervet I'm also stuck with 'Loading' as mentioned above.

guillaume-chervet commented 2 years ago

Does the 5.7.3-alpha0 version help you ? I set react as peerDependencies (as it have to be) If not I may need a sample of code on github or somewhere else where i can reproduce the error. I cannot manage to find a way to reproduce it.

slaterx commented 2 years ago

@guillaume-chervet it did not, the behaviour remains the same. I create a repo for you to reproduce the issue: https://github.com/slaterx/react-oidc-issue

Upon yarn start, you will see that the page is stuck on loading.

guillaume-chervet commented 2 years ago

Thank you it will help a lot. I check it out in few hours as soon as i am in front of a computer ^^

guillaume-chervet commented 2 years ago

npm install does seem to work. I think you have dependencies confict πŸ‘ image

I continue to test

slaterx commented 2 years ago

This is due to the issue reported here: https://github.com/facebook/create-react-app/issues/12279#issuecomment-1096724124

But running installation with yarn is a valid workaround πŸ‘

You can also run npm config set legacy-peer-deps true to use legacy peer dependencies (like in node v4) and get the installation completed nevertherless: image

guillaume-chervet commented 2 years ago

Hi @slaterx , I found it!

replace <OidcProvider configuration={configuration} configurationName={configurationName}> by <OidcProvider configuration={configuration}>

It will work :)

I have to add better message error for that case and in witOidcSecure, i found that configurationName and extras parameters are missing.

slaterx commented 2 years ago

Hi @guillaume-chervet, the issue persists after your proposed fix:

image

A good way to confirm the issue is by starting the local development server with HTTPS=true. This way, there's an SSL error on the browser and the worker code is not loaded. That way, the page loads without the OIDC code (which, obviously, means that all secure pages throw the default error - "Error authentication\nAn error occurred during authentication"):

image

Another relevant point is that this time I can't see anymore the service workers loaded (both on HTTP and HTTPS). I am running latest release (5.7.5), but when I was running the alpha version I could see the service workers being loaded.

We are a bit under pressure to move along with this implementation on our project, so if you could please help us pin point this bug it would be mostly appreciated! 😁

guillaume-chervet commented 2 years ago

I will send you a pull request on your demo as soon as i restarts my computer.

You have a also dependencies problem. Look at my error message with npm, i also removed some dependencies (tool dependencies)

guillaume-chervet commented 2 years ago

I send your a pull request https://github.com/slaterx/react-oidc-issue/pull/1

slaterx commented 2 years ago

Thanks for the hard work @guillaume-chervet and huge help, it's working with your changes!

I think that the takeaway here is that I'll have a URI context on my main SPA, (i.e. it won't be https://address/ but https://address/my/long/url/path and therefore my service worker JS will be on /org/repo/OidcServiceWorker.js, not on /OidcServiceWorker.js.

For the authentication stuff, the PR we did got the discovery phase done, but during the token validation after callback another CORS call is made and I get stuck where I was. Just to double confirm, assuming we are past the discovery phase, we cannot use the token received from /callback call straight away? We still need to call again the token endpoint to request the token one more time?

guillaume-chervet commented 2 years ago

I @slaterx , your oidc server should authorise cors request.

I think you have to look at the configuration on oidc server or contact guys that are in charge of it.

slaterx commented 2 years ago

Hi @guillaume-chervet!

We were hoping to use this library to implement OIDC without hosting a backend, but with the CORS limitation, we expected to be able to complete the authentication without the discovery phase, however the token retrieval also implies CORS.

Unfortunately, for security reasons, the auth provider is only providing implicit and authorization code without CORS. The only thing we can do is add the allowed redirect URIs, to make sure the provider will accept our authentication requests. We are able to authenticate and receive the callback with a valid code, but validating that code requires CORS again.

Thank you very much for your help, we will try to implement something that retrieves the token with a code and reply it with appropriate cookies to fulfil both auth provider and browser security.

You can close the incident.

guillaume-chervet commented 2 years ago

Thank you @slaterx ,

It will not fit with the security of your company, but technicaly you can proxify oidc http request from within your api.

I close the pull request. Thank you!