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

Use correct version of joi #392

Merged
merged 5 commits into from
Oct 30, 2024
Merged

Use correct version of joi #392

merged 5 commits into from
Oct 30, 2024

Conversation

sauntimo
Copy link
Contributor

@sauntimo sauntimo commented Oct 9, 2024

✏️ Changes

  • removes auth0-extension-hapi-tools dep and brings some required code in to the repo as the original repo (sub)depended on an incompatible version of joi
  • upgrade inert to @hapi/inert
  • override joi in blipp to the correct version
  • uses joi@17.12.2 which is now in canirequire (PR). This is the version which hapi-swagger@17.2.1 includes as a dependency in canirequire - see discussion here.

📷 Screenshots

If there were visual changes to the application with this change, please include before and after screenshots here. If it has animation, please use screen capture software like to make a gif.

🔗 References

🎯 Testing

Tested manually in prod-uk-1:

  • Node18 authz extension installed in prod-uk-1 tenant
  • Extension GUI running
  • Demozero login result showing authz rule executed successfully
  • Datadog prod log showing authz policy api request executed in node18

image

image

image

image

🚀 Deployment

🚫 This will be built locally and deployed manually when the appropriate webtask changes have made it to prod

@sauntimo sauntimo self-assigned this Oct 9, 2024
@sauntimo sauntimo changed the title Attempt to fix multiple joi schemas bug Use correct version of joi Oct 30, 2024
@@ -11,7 +11,7 @@ module.exports = `<!DOCTYPE html>
<link rel="stylesheet" type="text/css" href="https://cdn.auth0.com/styleguide/4.8.10/index.min.css" />
<link rel="stylesheet" type="text/css" href="https://cdn.auth0.com/manage/v0.3.1672/css/index.min.css">
<% if (assets.style) { %><link rel="stylesheet" type="text/css" href="<%= assets.style %>"><% } %>
<% if (assets.version) { %><link rel="stylesheet" type="text/css" href="//cdn.auth0.com/extensions/auth0-authz/assets/auth0-authz.ui.<%= assets.version %>.css"><% } %>
<% if (assets.version) { %><link rel="stylesheet" type="text/css" href="//cdn.auth0.com/extensions/develop/auth0-authz/assets/auth0-authz.ui.<%= assets.version %>.css"><% } %>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should changes to this file be reverted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oooh yes, great spot, thank you!!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 656be35

Copy link

@fbgoode fbgoode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested but have seen Tim's testing screenshot and it LGTM.

@sauntimo sauntimo marked this pull request as ready for review October 30, 2024 13:08
@sauntimo sauntimo merged commit 34ca629 into master Oct 30, 2024
6 checks passed
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 this pull request may close these issues.

2 participants