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

Skip Discovery Phase and Define Identity Endpoints #763

Closed
slaterx opened this issue Apr 28, 2022 · 31 comments
Closed

Skip Discovery Phase and Define Identity Endpoints #763

slaterx opened this issue Apr 28, 2022 · 31 comments

Comments

@slaterx
Copy link

slaterx commented Apr 28, 2022

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 (#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

  • Installed packages:
    "react": "^17.0.2",
    "react-csv": "^2.2.2",
    "react-dom": "^17.0.2",
@guillaume-chervet
Copy link
Contributor

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
Copy link
Author

slaterx commented Apr 28, 2022

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
Copy link
Contributor

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
Copy link
Contributor

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

@slaterx
Copy link
Author

slaterx commented Apr 28, 2022

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

@slaterx
Copy link
Author

slaterx commented Apr 28, 2022

@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
Copy link
Contributor

oO

@khyapate
Copy link

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

@guillaume-chervet
Copy link
Contributor

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
Copy link
Contributor

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

@khyapate
Copy link

khyapate commented May 3, 2022

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

@slaterx
Copy link
Author

slaterx commented May 4, 2022

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
Copy link
Contributor

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
Copy link
Author

slaterx commented May 4, 2022

@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
Copy link
Contributor

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
Copy link
Author

slaterx commented May 4, 2022

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
Copy link
Contributor

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

@khyapate
Copy link

khyapate commented May 4, 2022

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

@guillaume-chervet
Copy link
Contributor

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
Copy link
Author

slaterx commented May 5, 2022

@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
Copy link
Contributor

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
Copy link
Contributor

npm install does seem to work. I think you have dependencies confict 👍
image

I continue to test

@slaterx
Copy link
Author

slaterx commented May 5, 2022

This is due to the issue reported here: facebook/create-react-app#12279 (comment)

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
Copy link
Contributor

guillaume-chervet commented May 5, 2022

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
Copy link
Author

slaterx commented May 7, 2022

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
Copy link
Contributor

guillaume-chervet commented May 7, 2022

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
Copy link
Contributor

I send your a pull request slaterx/react-oidc-issue#1

@slaterx
Copy link
Author

slaterx commented May 8, 2022

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
Copy link
Contributor

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
Copy link
Author

slaterx commented May 9, 2022

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
Copy link
Contributor

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants