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

Replace references of uuid with crypto.randomUUID #12361

Draft
wants to merge 17 commits into
base: latest
Choose a base branch
from

Conversation

karinathomasbbc
Copy link
Contributor

Resolves JIRA [number]

Overall changes

A very high-level summary of easily-reproducible changes that can be understood by non-devs, and why these changes where made.

Code changes

  • A bullet point list of key code changes that have been made.

Testing

  1. List the steps used to test this PR.

Helpful Links

Add Links to useful resources related to this PR if applicable.

Coding Standards

Repository use guidelines

@karinathomasbbc karinathomasbbc self-assigned this Feb 4, 2025
cookieSetterSpy.mockClear();
const atUserId = getAtUserId();
it('should return the AT user id', () => {
Cookie.set('atuserid', '{ "val": "some-random-uuid" }');

Check warning

Code scanning / CodeQL

Clear text transmission of sensitive cookie Medium

Sensitive cookie sent without enforcing SSL encryption.

Copilot Autofix AI 5 days ago

To fix the problem, we need to ensure that the secure attribute is set when creating the cookie. This will ensure that the cookie is only transmitted over secure HTTPS connections. We can achieve this by modifying the Cookie.set calls to include the secure attribute in the options object.

Suggested changeset 1
src/app/lib/analyticsUtils/index.test.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/app/lib/analyticsUtils/index.test.js b/src/app/lib/analyticsUtils/index.test.js
--- a/src/app/lib/analyticsUtils/index.test.js
+++ b/src/app/lib/analyticsUtils/index.test.js
@@ -496,3 +496,3 @@
     it('should return the AT user id', () => {
-      Cookie.set('atuserid', '{ "val": "some-random-uuid" }');
+      Cookie.set('atuserid', '{ "val": "some-random-uuid" }', { secure: true });
       cookieSetterSpy.mockClear();
@@ -504,3 +504,3 @@
     it('should store the existing AT user id as a stringified JSON value in cookies again so that we update the cookie expiration date', () => {
-      Cookie.set('atuserid', '{ "val": "some-random-uuid" }');
+      Cookie.set('atuserid', '{ "val": "some-random-uuid" }', { secure: true });
       cookieSetterSpy.mockClear();
EOF
@@ -496,3 +496,3 @@
it('should return the AT user id', () => {
Cookie.set('atuserid', '{ "val": "some-random-uuid" }');
Cookie.set('atuserid', '{ "val": "some-random-uuid" }', { secure: true });
cookieSetterSpy.mockClear();
@@ -504,3 +504,3 @@
it('should store the existing AT user id as a stringified JSON value in cookies again so that we update the cookie expiration date', () => {
Cookie.set('atuserid', '{ "val": "some-random-uuid" }');
Cookie.set('atuserid', '{ "val": "some-random-uuid" }', { secure: true });
cookieSetterSpy.mockClear();
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
const [[cookieName, cookieValue, cookieOptions]] =
cookieSetterSpy.mock.calls;
it('should store the existing AT user id as a stringified JSON value in cookies again so that we update the cookie expiration date', () => {
Cookie.set('atuserid', '{ "val": "some-random-uuid" }');

Check warning

Code scanning / CodeQL

Clear text transmission of sensitive cookie Medium

Sensitive cookie sent without enforcing SSL encryption.

Copilot Autofix AI 5 days ago

To fix the problem, we need to ensure that the secure attribute is set when the cookie is created. This will enforce that the cookie is only transmitted over secure (HTTPS) connections. We can achieve this by modifying the Cookie.set calls to include the secure attribute in the options object.

Suggested changeset 1
src/app/lib/analyticsUtils/index.test.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/app/lib/analyticsUtils/index.test.js b/src/app/lib/analyticsUtils/index.test.js
--- a/src/app/lib/analyticsUtils/index.test.js
+++ b/src/app/lib/analyticsUtils/index.test.js
@@ -496,3 +496,3 @@
     it('should return the AT user id', () => {
-      Cookie.set('atuserid', '{ "val": "some-random-uuid" }');
+      Cookie.set('atuserid', '{ "val": "some-random-uuid" }', { secure: true });
       cookieSetterSpy.mockClear();
@@ -504,3 +504,3 @@
     it('should store the existing AT user id as a stringified JSON value in cookies again so that we update the cookie expiration date', () => {
-      Cookie.set('atuserid', '{ "val": "some-random-uuid" }');
+      Cookie.set('atuserid', '{ "val": "some-random-uuid" }', { secure: true });
       cookieSetterSpy.mockClear();
EOF
@@ -496,3 +496,3 @@
it('should return the AT user id', () => {
Cookie.set('atuserid', '{ "val": "some-random-uuid" }');
Cookie.set('atuserid', '{ "val": "some-random-uuid" }', { secure: true });
cookieSetterSpy.mockClear();
@@ -504,3 +504,3 @@
it('should store the existing AT user id as a stringified JSON value in cookies again so that we update the cookie expiration date', () => {
Cookie.set('atuserid', '{ "val": "some-random-uuid" }');
Cookie.set('atuserid', '{ "val": "some-random-uuid" }', { secure: true });
cookieSetterSpy.mockClear();
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
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.

1 participant