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
[Auth] Migrate app-backend
to use new auth services
#23719
Conversation
Changed Packages
|
a522684
to
a60db1a
Compare
Uffizzi Cluster |
e744c99
to
0224cfe
Compare
<OAuthRequestDialog /> | ||
<AppRouter> | ||
{/* For now, this is just a temporary solution to ensure the user remains logged in properly and has access to the main app content. */} | ||
<ExperimentalAppProtection> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems unnecessary to do manually, I would have placed it in the createRoot logic, but we can't have a dependency on the auth-react
package from the core-app-api
package, can we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The public path is not going to work, inject the mode instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert to an element instead of using a wrap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realised that the cookie refresh provider expects children, so it needs to be a wrap because of that.
packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts
Outdated
Show resolved
Hide resolved
91d100a
to
f60f00f
Compare
packages/backend-app-api/src/services/implementations/httpAuth/httpAuthServiceFactory.ts
Outdated
Show resolved
Hide resolved
f60f00f
to
4c7d816
Compare
plugins/auth-react/src/components/ExperimentalAppProtection/ExperimentalAppProtection.tsx
Outdated
Show resolved
Hide resolved
b8481e0
to
606687e
Compare
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
… AppIdentityProxy Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
… example Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mkay should be ready to go now 🤞
...backend-app-api/src/services/implementations/httpRouter/createCookieAuthRefreshMiddleware.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
packages/core-app-api/src/apis/implementations/IdentityApi/AppIdentityProxy.ts
Outdated
Show resolved
Hide resolved
exports.down = async function down(knex) { | ||
await knex.schema.alterTable('static_assets_cache', table => { | ||
table.dropIndex([], 'static_asset_cache_namespace_idx'); | ||
table.dropColumn('namespace'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you do these in the same transaction 🤔 maybe
plugins/auth-react/src/hooks/useCookieAuthRefresh/useCookieAuthRefresh.tsx
Outdated
Show resolved
Hide resolved
plugins/auth-react/src/components/CookieAuthRefreshProvider/CookieAuthRefreshProvider.test.tsx
Outdated
Show resolved
Hide resolved
packages/core-app-api/src/apis/implementations/IdentityApi/startCookieAuthRefresh.ts
Outdated
Show resolved
Hide resolved
packages/core-app-api/src/apis/implementations/IdentityApi/startCookieAuthRefresh.ts
Outdated
Show resolved
Hide resolved
packages/core-app-api/src/apis/implementations/IdentityApi/startCookieAuthRefresh.ts
Outdated
Show resolved
Hide resolved
const injectedConfigPath = | ||
appConfigs && (await injectConfig({ appConfigs, logger, staticDir })); | ||
|
||
await injectAppMode({ appMode, rootDir }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting that our disableConfigInjection
was there to cover readonly filesystems, and now this one also does writes - do we have to honor that flag for this purpose too?
Also, is it really not possible to do "live" injection of stuff in our app backend serving code? Even with like a middleware, that mutates the body before getting sent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had a live one but switched to this for simplicity, since that one needs to handle a couple of different cases. We could tweak the injection to only happen if it's not needed, but tbh I prefer the approach of adding it on the fly. Gonna leave that for a followup though
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
…lic bundle Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Thank you for contributing to Backstage! The changes in this pull request will be part of the |
Hey, I just made a Pull Request!
Migrate app-backend to use new auth APIs:
Remove issueUserCookie from HttpAuthService and move to auth-node✔️ Checklist
Signed-off-by
line in the message. (more info)