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

NIST-BRSKI policy app using claim cascade system #85

Merged
merged 110 commits into from
Sep 9, 2024

Conversation

A9-dev
Copy link
Contributor

@A9-dev A9-dev commented Sep 2, 2024

Setup

Install node dependencies with npm i in /packages/cahn_demo/client and /packages/cahn_demo/server.

Run the client in /packages/cahn_demo/client with npm run dev.
Run the server in /packages/cahn_demo/server with npm run dev.

(Sorry, couldn't get npm workspaces to work for some reason)

Ensure you have populated the /packages/cahn_demo/server/.env file, according to the template found in /packages/cahn_demo/server/.env_example.

Notes

There is much clean up to be done with this code. However, it should be functionally correct.

A9-dev and others added 30 commits August 12, 2024 16:27
Added various functions including VC upload, prolog queries, and email authentication
Note to self: things on the framework laptop are larger than they seem
…m' of https://github.com/nqminds/trustnetz into 70-implement-brski-policy-app-using-claim-cascade-system
…dpoint to check if a given device id can connect to network
@A9-dev A9-dev requested a review from AshleySetter September 2, 2024 14:09
@A9-dev A9-dev self-assigned this Sep 2, 2024
@A9-dev A9-dev linked an issue Sep 2, 2024 that may be closed by this pull request
Copy link
Contributor

@AshleySetter AshleySetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

App looks great, there's the bug we discussed on slack to fix, I've also found several places where client-side code could be extracted as react components to reduce code duplication.

);
};

export default withAuth(Page);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This page.js and the page.js for device_id look very similar, could we extract some of the common code as components to be reused across both pages?

Comment on lines 105 to 126
const data = {
"@context": ["https://www.w3.org/ns/credentials/v2"],
id: "urn:uuid:91cf3009-28ee-488e-8ae8-a751a289c8cb",
type: ["VerifiableCredential", "UserCredential"],
issuer: "urn:uuid:8bbabf61-758b-4bcb-8dab-4a4d1d493e25",
validFrom: "2024-07-25T19:23:24Z",
credentialSchema: {
id: "https://github.com/nqminds/ClaimCascade/blob/claim_verifier/packages/claim_verifier/user.yaml",
type: "JsonSchema",
},
credentialSubject: {
type: "fact",
schemaName: "device_trust",
id: uuidv4(),
timestamp: 1716287268891,
fact: {
device_id: params.device_id,
authoriser_id: emailAddress,
created_at: Date.now(),
},
},
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generate from schema?

);
};

export default withAuth(Page);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once again, very similar to the other page.js components for device and device_type, could we extract common components? Or even make the page a common component itself and only differentiate the 3 with props?

@A9-dev
Copy link
Contributor Author

A9-dev commented Sep 5, 2024

Changes

  • Styled table component
  • Schemas are accessible to the client (/client/src/schemas, publishing to npm felt overkill)
  • Made changes to manufacturer_trust schema to homogenise naming, used in sorting function
  • Trust VCs are built using the schema
  • TrustSubmissions component
  • Removed WASM initialisation function duplication

Notes

  • Other functions could be extracted, like the handleSubmitTrust for each page, but since they do slightly different things I'm on the fence whether it's better practice and safer to keep them separated or to make one more complicated function with more parameters like the endpoints used within the function.

@A9-dev A9-dev requested a review from AshleySetter September 5, 2024 14:13
Copy link
Contributor

@AshleySetter AshleySetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks for making those changes 👍

@A9-dev A9-dev merged commit e19d574 into main Sep 9, 2024
6 checks passed
@A9-dev A9-dev deleted the 70-implement-brski-policy-app-using-claim-cascade-system branch September 9, 2024 07:59
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.

Implement BRSKI Policy App using Claim Cascade System
2 participants