feat: allow sharing of secrets publicly + public page for secret sharing #1923

ShubhamPalriwala · 2024-06-05T13:00:10Z

Description 📣

Allows users to share secrets via Infisical without being authenticated thus helping in our GTM.
Authenticated users can do it from within their dashboard which allows them to keep a track of the secrets shared and their expiry details
New public page is at /shared/secret
Got rid of the Dragon 🐉

Public Page:

Shared Secret Page:

Type ✨

Bug fix
New feature
Breaking change
Documentation

Tests 🛠️

# Here's some code block to paste some code snippets

I have read the contributing guide, agreed and acknowledged the code of conduct. 📝

maidul98 · 2024-06-05T20:48:48Z

Bug: sometimes on first render, it will say expired even though it works again on next refresh.

This log might be related:

incoming request {"reqId":"req-3t","severity":"INFO","req":{"method":"GET","url":"/api/v1/secret-sharing/public/undefined?hashedHex=","hostname":"localhost:8080","remoteAddress":"192.168.96.1","remotePort":33220}}
infisical-dev-api              | [20:43:38.727] INFO (50): injectPermission: Injecting permissions for [permissionsForIdentity=9b5f450a-01aa-4581-bc3e-eeddd6fd9ff1] [type=user] {"severity":"INFO"}
infisical-dev-api              | [20:43:38.727] ERROR (50): [
infisical-dev-api              |   {
infisical-dev-api              |     "validation": "uuid",
infisical-dev-api              |     "code": "invalid_string",
infisical-dev-api              |     "message": "Invalid uuid",
infisical-dev-api              |     "path": [
infisical-dev-api              |       "id"
infisical-dev-api              |     ]
infisical-dev-api              |   }
infisical-dev-api              | ] {"reqId":"req-3t","severity":"ERROR"}
infisical-dev-api              |     err: {
infisical-dev-api              |       "type": "ZodError",
infisical-dev-api              |       "message": "[\n  {\n    \"validation\": \"uuid\",\n    \"code\": \"invalid_string\",\n    \"message\": \"Invalid uuid\",\n    \"path\": [\n      \"id\"\n    ]\n  }\n]",
infisical-dev-api              |       "stack":
infisical-dev-api              |           ZodError: [
infisical-dev-api              |             {
infisical-dev-api              |               "validation": "uuid",
infisical-dev-api              |               "code": "invalid_string",
infisical-dev-api              |               "message": "Invalid uuid",
infisical-dev-api              |               "path": [
infisical-dev-api              |                 "id"
infisical-dev-api              |               ]
infisical-dev-api              |             }
infisical-dev-api              |           ]
infisical-dev-api              |               at get error [as error] (/app/node_modules/zod/lib/types.js:43:31)
infisical-dev-api              |               at ZodObject.parse (/app/node_modules/zod/lib/types.js:143:22)
infisical-dev-api              |               at /app/src/server/plugins/fastify-zod.ts:2:2890
infisical-dev-api              |               at validateParam (/app/node_modules/fastify/lib/validation.js:110:36)
infisical-dev-api              |               at validate (/app/node_modules/fastify/lib/validation.js:132:20)
infisical-dev-api              |               at preValidationCallback (/app/node_modules/fastify/lib/handleRequest.js:91:25)
infisical-dev-api              |               at handler (/app/node_modules/fastify/lib/handleRequest.js:75:7)
infisical-dev-api              |               at handleRequest (/app/node_modules/fastify/lib/handleRequest.js:24:5)
infisical-dev-api              |               at runPreParsing (/app/node_modules/fastify/lib/route.js:609:5)
infisical-dev-api              |               at next (/app/node_modules/fastify/lib/hooks.js:233:9)
infisical-dev-api              |       "aggregateErrors": [
infisical-dev-api              |         {
infisical-dev-api              |           "type": "Object",
infisical-dev-api              |           "message": "Invalid uuid",
infisical-dev-api              |           "stack":
infisical-dev-api              |               
infisical-dev-api              |           "validation": "uuid",
infisical-dev-api              |           "code": "invalid_string",
infisical-dev-api              |           "path": [
infisical-dev-api              |             "id"
infisical-dev-api              |           ]
infisical-dev-api              |         }
infisical-dev-api              |       ],
infisical-dev-api              |       "issues": [
infisical-dev-api              |         {
infisical-dev-api              |           "validation": "uuid",
infisical-dev-api              |           "code": "invalid_string",
infisical-dev-api              |           "message": "Invalid uuid",
infisical-dev-api              |           "path": [
infisical-dev-api              |             "id"
infisical-dev-api              |           ]
infisical-dev-api              |         }
infisical-dev-api              |       ],
infisical-dev-api              |       "name": "ZodError",
infisical-dev-api              |       "statusCode": 400,
infisical-dev-api              |       "code": "FST_ERR_VALIDATION",
infisical-dev-api              |       "validationContext": "params"
infisical-dev-api              |     }

Side note, i think it will be better if the error is more specific. Because is the secret really not found or did it actually expire? As a user, if it is expired, i can tell the sender to resend it but if it is not found then i would think they gave me the wrong link

maidul98

Left some comments. I think the UI need some fix up. @vmatsiiako will help with it.

Small nit, i would rename the page URL from /shared/secret/ to /share-secret

backend/src/server/routes/v1/secret-sharing-router.ts

ShubhamPalriwala · 2024-06-06T08:51:50Z

Side note, i think it will be better if the error is more specific. Because is the secret really not found or did it actually expire? As a user, if it is expired, i can tell the sender to resend it but if it is not found then i would think they gave me the wrong link

We actually had it initially but then from a security + brute force concern, we limited this to treat invalid secrets the same way as expired secrets so that one cannot try decoding it by brute forcing it. And the current setup deletes the secrets once they're expired on views and at 0 GMT when time bound expires so we technically do not store expired secrets as such.

ShubhamPalriwala · 2024-06-06T08:53:41Z

About the undefined error, I was not able to reproduce it :/ Can you try once now with the latest commit to see if you still see it? PS: @vmatsiiako will take a look at the UI side of this PR sometime tomorrow.

Ready for review again 🙏🏼

sheensantoscapadngan · 2024-06-07T16:45:25Z

backend/src/db/migrations/20240605074539_make-secret-sharing-public.ts

+      t.uuid("orgId").nullable().alter();
+      t.uuid("userId").nullable().alter();


I think we should be adding hasColumn checks for these ones?

I did not do that initially because the orgId will always be there since its a part of the initial schema, so I'm not sure if a case for this could be possible. But I'll add it now.

sheensantoscapadngan · 2024-06-07T16:45:32Z

backend/src/db/migrations/20240605074539_make-secret-sharing-public.ts

+      t.uuid("orgId").notNullable().alter();
+      t.uuid("userId").notNullable().alter();


here as well

sheensantoscapadngan · 2024-06-07T16:48:59Z

backend/src/server/routes/v1/secret-sharing-router.ts

+        iv,
+        tag,
+        hashedHex,
+        expiresAt: new Date(expiresAt),


I think we should be validating the value of the expiresAt being passed to this endpoint so that no malicious user could fill up our records with non-expiring secrets (like they set it to 10 years from now or something)

or are we already doing that?

Thanks, limiting it to 30 days for now.

backend/src/server/routes/v1/secret-sharing-router.ts

maidul98

Somewhere, we keep making these requests, we should catch and avoid these requests when the param is undefined:

{"reqId":"req-13","severity":"INFO","req":{"method":"GET","url":"/api/v1/secret-sharing/public/undefined?hashedHex=","hostname":"localhost:8080","remoteAddress":"192.168.144.1","remotePort":50246}}

The text area needs some work, you can only click on a specific area. We should also show the scroll bar if we haven't yet because for long shares, people may not know. Also i don't remember, did we set a max context limit that can be shared?
Not sure where the share your own secret button went, it was there but now i don't see it anymore

ShubhamPalriwala · 2024-06-07T19:49:43Z

Somewhere, we keep making these requests, we should catch and avoid these requests when the param is undefined:
{"reqId":"req-13","severity":"INFO","req":{"method":"GET","url":"/api/v1/secret-sharing/public/undefined?hashedHex=","hostname":"localhost:8080","remoteAddress":"192.168.144.1","remotePort":50246}}

Done

The text area needs some work, you can only click on a specific area. We should also show the scroll bar if we haven't yet because for long shares, people may not know. Also i don't remember, did we set a max context limit that can be shared?

Tried debugging Text area issue but couldn't make aby significant progress
I think Mac prevents showing scrollbars as default unless scrolled, but apart from that I could nit find a straightforward tailwind class to help us achieve that.
We've set max content limit to 10k characters, but that's only on client side. A user can still hit us with a huge payload with the API call directly. Should we limit this? And by how much?

Not sure where the share your own secret button went, it was there but now i don't see it anymore

It's now of the outline variant and available as shown below:

…ven-for-non-infisical-users

mintlify bot deployed to staging June 5, 2024 13:04 View deployment

ShubhamPalriwala requested a review from maidul98 June 5, 2024 14:52

maidul98 reviewed Jun 5, 2024

View reviewed changes

backend/src/server/routes/v1/secret-sharing-router.ts Outdated Show resolved Hide resolved

backend/src/server/routes/v1/secret-sharing-router.ts Show resolved Hide resolved

ShubhamPalriwala requested a review from sheensantoscapadngan June 7, 2024 16:02

sheensantoscapadngan reviewed Jun 7, 2024

View reviewed changes

maidul98 reviewed Jun 7, 2024

View reviewed changes

ShubhamPalriwala and others added 10 commits June 10, 2024 07:59

feat: allow sharing of secrets publicly

5df53a2

feat: public page to share secrets

d743537

docs: update screenshot & mention public usage

6c0ab43

fix: resolve feedback + new endpoint + new write rate limit

0e86d55

feat: public page has direct secret creation (no modal))

a6e71c9

updated design of secret sharing

263d321

feat: change ui for new secret and add button for existing shared secret

24b8b64

fix: move share secret button above textbox

43a3731

fix nits

1a6b710

feat: limit expiry to 1 month & minor ui fixes

f6e4446

ShubhamPalriwala force-pushed the shubham/eng-984-make-secret-sharing-public-even-for-non-infisical-users branch from aa69f5b to f6e4446 Compare June 10, 2024 02:35

docs: update image

9e4b248

ShubhamPalriwala requested a review from maidul98 June 10, 2024 02:40

ShubhamPalriwala and others added 3 commits June 10, 2024 10:34

fix: add limit to character length in api as well

abd4b41

nits

de3e23e

Merge branch 'main' into shubham/eng-984-make-secret-sharing-public-e…

39df6ce

…ven-for-non-infisical-users

maidul98 merged commit c9f7f64 into main Jun 14, 2024
6 of 7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow sharing of secrets publicly + public page for secret sharing #1923

feat: allow sharing of secrets publicly + public page for secret sharing #1923

ShubhamPalriwala commented Jun 5, 2024

maidul98 commented Jun 5, 2024 •

edited

Loading

maidul98 left a comment

ShubhamPalriwala commented Jun 6, 2024

ShubhamPalriwala commented Jun 6, 2024

sheensantoscapadngan Jun 7, 2024

ShubhamPalriwala Jun 7, 2024

sheensantoscapadngan Jun 7, 2024

ShubhamPalriwala Jun 7, 2024

sheensantoscapadngan Jun 7, 2024

maidul98 Jun 7, 2024

ShubhamPalriwala Jun 7, 2024

maidul98 left a comment •

edited

Loading

ShubhamPalriwala commented Jun 7, 2024 •

edited

Loading

		t.uuid("orgId").nullable().alter();
		t.uuid("userId").nullable().alter();

		t.uuid("orgId").notNullable().alter();
		t.uuid("userId").notNullable().alter();

feat: allow sharing of secrets publicly + public page for secret sharing #1923

feat: allow sharing of secrets publicly + public page for secret sharing #1923

Conversation

ShubhamPalriwala commented Jun 5, 2024

Description 📣

Public Page:

Shared Secret Page:

Type ✨

Tests 🛠️

maidul98 commented Jun 5, 2024 • edited Loading

maidul98 left a comment

Choose a reason for hiding this comment

ShubhamPalriwala commented Jun 6, 2024

ShubhamPalriwala commented Jun 6, 2024

sheensantoscapadngan Jun 7, 2024

Choose a reason for hiding this comment

ShubhamPalriwala Jun 7, 2024

Choose a reason for hiding this comment

sheensantoscapadngan Jun 7, 2024

Choose a reason for hiding this comment

ShubhamPalriwala Jun 7, 2024

Choose a reason for hiding this comment

sheensantoscapadngan Jun 7, 2024

Choose a reason for hiding this comment

maidul98 Jun 7, 2024

Choose a reason for hiding this comment

ShubhamPalriwala Jun 7, 2024

Choose a reason for hiding this comment

maidul98 left a comment • edited Loading

Choose a reason for hiding this comment

ShubhamPalriwala commented Jun 7, 2024 • edited Loading

maidul98 commented Jun 5, 2024 •

edited

Loading

maidul98 left a comment •

edited

Loading

ShubhamPalriwala commented Jun 7, 2024 •

edited

Loading