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

bug: consecutive source-hub acp operations fail for http client due to an identity auth error #3065

Closed
shahzadlone opened this issue Sep 24, 2024 · 1 comment · Fixed by #3068
Assignees
Labels
area/acp Related to the acp (access control) system bug Something isn't working
Milestone

Comments

@shahzadlone
Copy link
Member

shahzadlone commented Sep 24, 2024

While implementing #2762 in #2907 I came across a bug where calling a sourcehub operation on two different nodes failed i.e. attempting to share a document by the same owner with a 2nd owner on each 2 different nodes causes the first node's operation to succeed and the second nodes operation to fail.

My hunch was that this is not specific to the relationship sharing feature, as this is only an issue for http client with sourcehub.

I tested a basic example of creating 2 private documents using the same identity (which uses acp identity operations underneath ofc.), one on node 1 and second on node 2 using sourcehub with http client. As suspected the tests fail, hence this is an existing bug.

Here is a test that documents the failure:

func TestDEBUG(t *testing.T) {
	expectedPolicyID := "fc56b7509c20ac8ce682b3b9b4fdaad868a9c70dda6ec16720298be64f16e9a4"

	test := testUtils.TestCase{

		Description: "Test source-hub acp with http, consecutive operations on different nodes, identity bug",

		SupportedACPTypes: immutable.Some(
			[]testUtils.ACPType{
				testUtils.SourceHubACPType,
			},
		),

		Actions: []any{
			testUtils.RandomNetworkingConfig(),

			testUtils.RandomNetworkingConfig(),

			testUtils.AddPolicy{

				Identity: immutable.Some(1),

				Policy: `
                    name: Test Policy

                    description: A Policy

                    actor:
                      name: actor

                    resources:
                      users:
                        permissions:
                          read:
                            expr: owner + reader + writer

                          write:
                            expr: owner + writer

                          nothing:
                            expr: dummy

                        relations:
                          owner:
                            types:
                              - actor

                          reader:
                            types:
                              - actor

                          writer:
                            types:
                              - actor

                          admin:
                            manages:
                              - reader
                            types:
                              - actor

                          dummy:
                            types:
                              - actor
                `,

				ExpectedPolicyID: expectedPolicyID,
			},

			testUtils.SchemaUpdate{
				Schema: fmt.Sprintf(`
						type Users @policy(
							id: "%s",
							resource: "users"
						) {
							name: String
							age: Int
						}
					`,
					expectedPolicyID,
				),
			},

			testUtils.CreateDoc{
				Identity: immutable.Some(1),

				NodeID: immutable.Some(0),

				CollectionID: 0,

				DocMap: map[string]any{
					"name": "Shahzad",
				},
			},

			testUtils.CreateDoc{
				Identity: immutable.Some(1),

				NodeID: immutable.Some(1),

				CollectionID: 0,

				DocMap: map[string]any{
					"name": "Shahzad Lone",
				},
			},
		},
	}

	testUtils.ExecuteTestCase(t, test)
}

Output:

Error: Received unexpected error: 403: forbidden
@shahzadlone shahzadlone added bug Something isn't working priority/high area/acp Related to the acp (access control) system labels Sep 24, 2024
@shahzadlone shahzadlone added this to the DefraDB v0.14 milestone Sep 24, 2024
@shahzadlone shahzadlone self-assigned this Sep 24, 2024
@shahzadlone shahzadlone changed the title bug: consecutive source-hub acp operations fail for http client due to an identity error bug: consecutive source-hub acp operations fail for http client due to an identity auth error Sep 24, 2024
@shahzadlone
Copy link
Member Author

Bumped down in priority, I think this is not a prod issue, might just be a testing framework issue the way we reuse identity bearer tokens between nodes, but they should have different tokens I believe due to different audience values (i.e. node host values) which might be causing this.

ChrisBQu pushed a commit to ChrisBQu/defradb that referenced this issue Feb 21, 2025
…rk#3068)

## Relevant issue(s)
Resolves sourcenetwork#3065 

## Description
The main bug was only visible on sourcehub acp using http, due to the
identity being copied with the audience value of another node's host
failing authentication (the bearer tokens should be unique using correct
node's audience). The biggest issue was the way we use `getNodes` and
`getNodeCollections`. I would be in favor of completely removing them as
they are more troublesome than the utility they provide.

- First commit documents the bug
- Some utility functions were overwriting and producing the wrong node
index
- Forbidden bug happening on sourcehub<>http test run:
https://github.com/sourcenetwork/defradb/actions/runs/10930293192/job/30342883535?pr=2907

### Future:
- Resolve import/export documentation and implementation if different
(sourcenetwork#3067)
- Should likely clean this test utils up and make helper methods to
avoid code duplication
(sourcenetwork#3069)
- Likely should remove all usages of `getNodeCollections`
(sourcenetwork#3069)
- Likely should remove all usages of `getNodes`
(sourcenetwork#3069)


## How has this been tested?
- Very painfully haha, had to install Linux bare-metal to investigate
the first bug (sourcehub doesn't build on wsl for me) that was only
occurring on sourcehub acp using http, due to the identity being copied
with the audience value of another node the way we use `getNodes` and
`getNodeCollections`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/acp Related to the acp (access control) system bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant