-
Notifications
You must be signed in to change notification settings - Fork 72
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
Add Google Tag Manager and Privacy Center ENV vars to sample app, plus the ability to pass ENV vars to both privacy center and sample app during fides deploy
via .env
#2949
Conversation
Passing run #2250 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #2949 +/- ##
=======================================
Coverage 87.13% 87.13%
=======================================
Files 312 312
Lines 18659 18659
Branches 2377 2377
=======================================
Hits 16259 16259
Misses 1980 1980
Partials 420 420 ☔ View full report in Codecov by Sentry. |
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.
Overall this looks pretty reasonable! I know it's just a "sample app" but I do want to investigate the potential XSS possibility
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.
Suggestion for XSS defense!
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 looks good now, just add an entry to CHANGELOG and this should be ready to go. Thanks!
40aadf6
to
8ee5e57
Compare
database: DATABASE_DB || "postgres_example", | ||
host: host || DATABASE_HOST || "localhost", | ||
port: Number(port || DATABASE_PORT || 5432), | ||
user: user || DATABASE_USER || "postgres", |
Check failure
Code scanning / CodeQL
Hard-coded credentials
host: host || DATABASE_HOST || "localhost", | ||
port: Number(port || DATABASE_PORT || 5432), | ||
user: user || DATABASE_USER || "postgres", | ||
password: password || DATABASE_PASSWORD || "postgres", |
Check failure
Code scanning / CodeQL
Hard-coded credentials
fides deploy
via .env
Updated this branch to the latest To make those ENV vars easy to access when testing locally, this also passes the |
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.
To actually use this, we should also be calling Fides.gtm()
, duh! 🙃
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.
OK, LGTM now!
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.
OK, LGTM now!
Closes #3347
Closes #29
Code Changes
FIDES_SAMPLE_APP__GOOGLE_TAG_MANAGER_CONTAINER_ID
FIDES_SAMPLE_APP__PRIVACY_CENTER_URL
.env
values tofides-privacy-center
andfides-sample-app
images when runningfides deploy
README.md
forfides-sample-app
Fides.gtm()
to enable the Fides <> GTM integrationSteps to Confirm
.env
file withFIDES_SAMPLE_APP__GOOGLE_TAG_MANAGER_CONTAINER_ID=GTM-MYGTMID
nox -s "fides_env(test)"
Pre-Merge Checklist
CHANGELOG.md
Description Of Changes
This adds an (optional) GTM container tag to the Cookie House sample app for testing, and then adds additional flexibility to the advanced usage of
fides deploy
by allowing the user to edit their local.env
file and provide extra configuration for the various images.This also adds a configurable option for
FIDES_SAMPLE_APP__PRIVACY_CENTER_URL
so that this isn't always hardcoded to http://localhost:3001 👍