feat: update to support array middleware by chmaltsp · Pull Request #200 · seamapi/fake-seam-connect

chmaltsp · 2024-06-14T00:31:53Z

No description provided.

seambot · 2024-06-14T00:32:50Z

Bundle dist size: 26M

razor-x

Got about 1/2 way though.

razor-x · 2024-06-18T16:12:38Z

Let's try to replace all cases of the cst abbreviation with client session or client session token. So name this withClientSession

razor-x · 2024-06-18T16:12:53Z


 import { withSimulatedOutage } from "./with-simulated-outage.ts"

 export const withCst: Middleware<


Suggested change

export const withCst: Middleware<

export const withClientSession: Middleware<

razor-x · 2024-06-18T16:14:32Z


  const relevant_cs = req.db.client_sessions.find((cs) =>
-    cs.connect_webview_ids.includes(connect_webview.connect_webview_id),
+    cs.workspace_id.includes(connect_webview.workspace_id),


Why was this logic updated? The original logic looks correct.

The original logic was not working because the client session actually never got updated with the connect_webview_id. I'm not sure how that was working in the first place and I think it was skipping over the relevant_cs most of the time.

razor-x · 2024-06-18T16:16:04Z

    manufacturer,
  } = req.commonParams
-  const { workspace_id } = req.auth
+  const { workspace_id } = req.auth as { workspace_id: string }


Why did we loose type safety?

razor-x · 2024-06-18T16:29:22Z

 export default withRouteSpec({
  methods: ["POST"],
-  auth: "cst_ak_pk",
+  auth: ["client_session"],


Suggested change

auth: ["client_session"],

auth: ["api_key", "console_session"],

razor-x · 2024-06-18T16:37:45Z

@@ -0,0 +1,42 @@
+export type AuthenticatedRequest = Request & {


Generally at Seam, we do not write code in index files (existing cases are legacy examples). Types should be defined alongside the module needs them, or in this case we could simply name this authenticated-request.ts

Co-authored-by: Evan Sosenko <evan@getseam.com>

stale · 2024-06-21T21:35:06Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

razor-x

Made it through the rest 😄

razor-x · 2024-06-24T21:06:44Z


-  if (req.auth.auth_mode === "publishable_key" && user_identifier_key == null) {
+  if (
+    req.auth.type === "client_session" &&


Client sessions cannot create more client sessions, this check only applies to publishable_key auth.

Based on the implementation in seam-connect only client_sessions have the method property in the auth middleware with publishable_key as an option, is this incorrect?

The endpoint should always return 403 if client session auth.

razor-x · 2024-06-24T21:09:15Z

    db,
  } = await getTestServer(t)

+  const cs = db.client_sessions.find((cs) => cs.token === ws2.cst)


Aren't only console sessions JWT? A client session cannot reset a sandbox workspace. What is this testing?

razor-x · 2024-06-24T21:10:19Z

    connect_webview_ids: [cw.connect_webview_id],
    connected_account_ids: [ca.connected_account_id],
    user_identifier_key: "seed_client_session_user",
+    user_identity_ids: ["john_user_identity_id"],


Where does this id come from? I would not expect it to be hard coded here.

This value is available when we seed the database in

fake-seam-connect/src/lib/database/seed.ts

Line 43 in d163139

john_user_identity_id: "john_user_identity_id",

so I was just adding to this client session as well

razor-x · 2024-06-24T21:14:53Z

-      auth_mode: "access_token"
+export const withAccessToken =
+  <RequiresWorkspaceId extends boolean>({
+    require_workspace_id,


Suggested change

require_workspace_id,

is_workspace_id_required,

can make this change, but was trying to align naming with current convention in seam-connect, is it worth drifting away?

Sure, if some of this is copy/paste from seam connect, then it's ok to follow that. I can't really know the difference in review, so I just have to flag things that look like they break convention. You can apply best judgement.

razor-x · 2024-06-24T21:15:49Z


-  if (short_token == null || long_token == null)
-    return res.status(400).end("malformed token")
+  if (!is_cst) {


Suggested change

if (!is_cst) {

if (!is_client_session_token) {

razor-x · 2024-06-24T21:18:45Z

+      workspace_id_from_header !== null &&
+      typeof workspace_id_from_header !== "undefined" &&


This is actually the simplest way to do safe null checks

Suggested change

workspace_id_from_header !== null &&

typeof workspace_id_from_header !== "undefined" &&

workspace_id_from_header != null

(or workspace_id_from_header == null for the inverse)

razor-x · 2024-06-24T21:19:28Z

+      }
+
+      if (require_workspace_id) {
+        ;(req.auth as Extract<


Similar comment as before.

stale · 2024-06-27T22:54:33Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale · 2024-07-07T01:36:52Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

razor-x · 2024-07-09T22:34:08Z

Everything looks good expect this change: https://github.com/seamapi/fake-seam-connect/pull/200/files#r1651613761

Maybe we can just revert it and add some more tests here for api key and pat auth methods? https://github.com/seamapi/fake-seam-connect/blob/main/test/api/client_sessions/create.test.ts

stale · 2024-07-13T05:32:24Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

chmaltsp · 2024-08-01T18:21:18Z

Everything looks good expect this change: https://github.com/seamapi/fake-seam-connect/pull/200/files#r1651613761

Maybe we can just revert it and add some more tests here for api key and pat auth methods? https://github.com/seamapi/fake-seam-connect/blob/main/test/api/client_sessions/create.test.ts

Sorry this one kind of fell off the radar. I'd like to get this over the finish line. I'm not sure what you mean by reverting it, are we going to support both auth_method schema as well as the new one? Happy to change it to a 403.

razor-x · 2024-08-01T19:50:06Z

Everything looks good expect this change: #200 (files)
Maybe we can just revert it and add some more tests here for api key and pat auth methods? main/test/api/client_sessions/create.test.ts

Sorry this one kind of fell off the radar. I'd like to get this over the finish line. I'm not sure what you mean by reverting it, are we going to support both auth_method schema as well as the new one? Happy to change it to a 403.

Sure. The issue is still the one from this comment: #200 (comment)

The endpoint auth method in this PR does not match the behavior for the actual endpoint.

…eam-connect into pc/update-auth-middleware

razor-x · 2024-08-05T22:24:18Z

@chmaltsp I tried to test out the changes in the SDK but I'm getting errors. I think we are close but either I'm doing something wrong with passing the tokens or the fake is not authorizing them properly: seamapi/javascript-http#150

feat: update to support array middleware

7387a53

vercel Bot deployed to Preview June 14, 2024 00:32 View deployment

wip: add session auth

cb042ba

vercel Bot deployed to Preview June 15, 2024 01:33 View deployment

fix: update middlewares and update tests

565f6c8

chmaltsp marked this pull request as ready for review June 18, 2024 01:01

chmaltsp requested a review from razor-x as a code owner June 18, 2024 01:01

vercel Bot deployed to Preview June 18, 2024 01:01 View deployment

fix: eslint fixes

2990141

vercel Bot deployed to Preview June 18, 2024 01:05 View deployment

fix: fix typecheck

0e653c0

vercel Bot deployed to Preview June 18, 2024 16:23 View deployment

chmaltsp and others added 2 commits June 18, 2024 09:53

chore: fix deps

a4bb9f1

ci: Generate code

b62173b

vercel Bot deployed to Preview June 18, 2024 16:55 View deployment

razor-x reviewed Jun 18, 2024

View reviewed changes

Update test/api/devices/list.test.ts

6a20b4b

Co-authored-by: Evan Sosenko <evan@getseam.com>

vercel Bot deployed to Preview June 18, 2024 18:58 View deployment

fix: address comments

073ab1e

vercel Bot deployed to Preview June 18, 2024 21:01 View deployment

stale Bot added the stale label Jun 21, 2024

stale Bot closed this Jun 22, 2024

razor-x reopened this Jun 24, 2024

stale Bot removed the stale label Jun 24, 2024

razor-x reviewed Jun 24, 2024

View reviewed changes

stale Bot added the stale label Jun 27, 2024

stale Bot closed this Jun 29, 2024

ci: Generate code

b878dc5

vercel Bot deployed to Preview July 3, 2024 21:57 View deployment

chore: fix typecheck

6f5e404

vercel Bot deployed to Preview July 4, 2024 00:23 View deployment

ci: Format code

922e0cc

vercel Bot deployed to Preview July 4, 2024 00:24 View deployment

stale Bot added the stale label Jul 7, 2024

stale Bot closed this Jul 8, 2024

chmaltsp reopened this Jul 8, 2024

stale Bot removed the stale label Jul 8, 2024

chmaltsp requested a review from razor-x July 8, 2024 18:26

stale Bot added the stale label Jul 13, 2024

stale Bot closed this Jul 14, 2024

razor-x reopened this Aug 1, 2024

stale Bot closed this Aug 2, 2024

chmaltsp added 2 commits August 2, 2024 17:46

fix: update tests for client_session create

37dd1c1

Merge branch 'pc/update-auth-middleware' of github.com:seamapi/fake-s…

cd3ae0e

…eam-connect into pc/update-auth-middleware

razor-x reopened this Aug 5, 2024

razor-x approved these changes Aug 5, 2024

View reviewed changes

fix: fix typecheck

4fbd91d

vercel Bot deployed to Preview August 5, 2024 20:31 View deployment

chmaltsp merged commit 3399244 into main Aug 5, 2024

chmaltsp deleted the pc/update-auth-middleware branch August 5, 2024 21:08

razor-x mentioned this pull request Aug 30, 2024

test: Test other auth methods against fake-seam-connect seamapi/javascript-http#150

Merged


		import { withSimulatedOutage } from "./with-simulated-outage.ts"

		export const withCst: Middleware<

	export const withCst: Middleware<
	export const withClientSession: Middleware<

	auth: ["client_session"],
	auth: ["api_key", "console_session"],

		@@ -0,0 +1,42 @@
		export type AuthenticatedRequest = Request & {

		workspace_id_from_header !== null &&
		typeof workspace_id_from_header !== "undefined" &&

	workspace_id_from_header !== null &&
	typeof workspace_id_from_header !== "undefined" &&
	workspace_id_from_header != null

Conversation

chmaltsp commented Jun 14, 2024

Uh oh!

seambot commented Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

razor-x left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stale Bot commented Jun 21, 2024

Uh oh!

razor-x left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stale Bot commented Jun 27, 2024

Uh oh!

stale Bot commented Jul 7, 2024

Uh oh!

razor-x commented Jul 9, 2024

Uh oh!

stale Bot commented Jul 13, 2024

Uh oh!

chmaltsp commented Aug 1, 2024

Uh oh!

razor-x commented Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

razor-x commented Aug 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

seambot commented Jun 14, 2024 •

edited

Loading

razor-x commented Aug 1, 2024 •

edited

Loading