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.

Sign up for GitHub

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

feat: relocation of ctype config #78

Merged
merged 15 commits into from
Mar 13, 2024
Merged

feat: relocation of ctype config #78

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
973a34f
config ctype to .env
Jan 31, 2024
1302a8f
feat: making the requested changes
Feb 5, 2024
fcb7054
feat: ctype table addation
Feb 5, 2024
33828af
remove default values
aybarsayan Feb 13, 2024
99ad5a6
feat: mapfix
aybarsayan Feb 14, 2024
4fe1b4e
fix: pr changes
aybarsayan Feb 29, 2024
8c89cc8
fix: genesisEnv
aybarsayan Feb 29, 2024
177b978
feat: ability to use more than one ctype
aybarsayan Mar 13, 2024
6e32123
fix: lint
aybarsayan Mar 13, 2024
4868513
fix: some Review Changes
aybarsayan Mar 13, 2024
e1e82a4
fix: log remove
aybarsayan Mar 13, 2024
692758b
fix: export
aybarsayan Mar 13, 2024
d716392
fix: Ctype type
aybarsayan Mar 13, 2024
36833d7
Merge branch 'main' of https://github.com/aybarsayan/web3-login-demo
aybarsayan Mar 13, 2024
4558746
fix: lint fixes
aybarsayan Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
WSS_ADDRESS=wss://peregrine.kilt.io/;
WSS_ADDRESS=wss://peregrine.kilt.io/
FRONTEND_PORT=6565
BACKEND_PORT=2525
DAPP_ACCOUNT_MNEMONIC=
DAPP_DID_MNEMONIC=
DAPP_DID_URI=
DAPP_NAME=
JWT_SIGNER_SECRET=
CTYPE_HASH=0x3291bb126e33b4862d421bfaa1d2f272e6cdfc4f96658988fbcffea8914bd9ac
rompemoldes marked this conversation as resolved.
Show resolved Hide resolved
TRUSTED_ATTESTERS=did:kilt:4pehddkhEanexVTTzWAtrrfo2R7xPnePpuiJLC7shQU894aY
REQUIRED_PROPERTIES=Email
17 changes: 17 additions & 0 deletions README.md
View file
Open in desktop
aybarsayan marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,23 @@ The following variables are required:
- `DAPP_DID_URI` = _This is the URI of the Kilt DID that identifies your dApp_
- `DAPP_NAME` = _This should be a custom name for your dApp_
- `JWT_SIGNER_SECRET` = _This is secret key (string) that signs the Json-Web-Tokens before saving them in the Cookies_
- `CTYPE_HASH` = _This is type of credential (CType) your dApp will request from users for login_
- `TRUSTED_ATTESTERS` = _This is the DID that identifies the attestors whose attestations are accepted_
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `TRUSTED_ATTESTERS` = _This is the DID that identifies the attestors whose attestations are accepted_
- `TRUSTED_ATTESTERS` = _This is a list of attesters DIDs (c.s.v.`,`). Only credentials issued by them will be considered valid_

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done at: 4fe1b4e

- `REQUIRED_PROPERTIES` = _These are the required properties that identify the CType_
Copy link
Contributor

@rompemoldes rompemoldes Feb 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
- `REQUIRED_PROPERTIES` = _These are the required properties that identify the CType_
- `REQUIRED_PROPERTIES` = _This is a subset of CType properties (c.s.v.`,`) required to be exposed on credential presentation_

This properties do not define the CType. They are just the properties out of the CType that we are asking for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done at: 4fe1b4e


To proceed, you may select the desired credential through the CType Environment Variables listed below, allowing you to tailor the authentication process to your needs.

| Socail KYC | CTYPE_HASH | TRUSTED_ATTESTERS(spiritnet) | TRUSTED_ATTESTERS(peregrine) | REQUIRED_PROPERTIES |
Copy link
Contributor

Choose a reason for hiding this comment

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

The whole table just has some useful information about some recommended cTypes. I would give it an according description and also a title (via ###on markdown). Maybe "Info about example/recommended cTypes" or something like that.

Please, also signalise somehow that we only recommend SocialKYC and the types of credentials it issues.
This is also a good place to explain why the cType-hash is the same for both chains.

The first row is not great. The first column should be "Title", not "SocialKYC".

Both TRUSTED_ATTESTERS(spiritnet) and TRUSTED_ATTESTERS(peregrine) should rather be SocialKYC on Spiritnet or on SocialKYC on Peregrine.

Maybe you can first list the two SocialKYC DIDs and then add references to them on the table. Then you can leave the title Trusted Attesters. That would be more fitting, if later we decide to add an external attester to this list.

You will have to make them subtitles to be able to reference them, like this:

####SocialKYC on the Production: did:kilt:...

And then inside the table (prod. SYKC)[#SocialKYC on the Production]

And on the properties column is maybe better to include all properties and let them decide. This would make the "required" part false. But the columns name really don't need to be the name of the CONSTANTS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done at: 4fe1b4e

|------------|--------------------------------------------------------------------|-----------------------------------------------------------|-----------------------------------------------------------|---------------------|
| Email | 0x3291bb126e33b4862d421bfaa1d2f272e6cdfc4f96658988fbcffea8914bd9ac | did:kilt:4pnfkRn5UurBJTW92d9TaVLR2CqJdY4z5HPjrEbpGyBykare | did:kilt:4pehddkhEanexVTTzWAtrrfo2R7xPnePpuiJLC7shQU894aY | Email |
| Twitter | 0x47d04c42bdf7fdd3fc5a194bcaa367b2f4766a6b16ae3df628927656d818f420 | did:kilt:4pnfkRn5UurBJTW92d9TaVLR2CqJdY4z5HPjrEbpGyBykare | did:kilt:4pehddkhEanexVTTzWAtrrfo2R7xPnePpuiJLC7shQU894aY | Twitter |
| Discord | 0xd8c61a235204cb9e3c6acb1898d78880488846a7247d325b833243b46d923abe | did:kilt:4pnfkRn5UurBJTW92d9TaVLR2CqJdY4z5HPjrEbpGyBykare | did:kilt:4pehddkhEanexVTTzWAtrrfo2R7xPnePpuiJLC7shQU894aY | Discord |
| GitHub | 0xad52bd7a8bd8a52e03181a99d2743e00d0a5e96fdc0182626655fcf0c0a776d0 | did:kilt:4pnfkRn5UurBJTW92d9TaVLR2CqJdY4z5HPjrEbpGyBykare | did:kilt:4pehddkhEanexVTTzWAtrrfo2R7xPnePpuiJLC7shQU894aY | GitHub |
| Twitch | 0x568ec5ffd7771c4677a5470771adcdea1ea4d6b566f060dc419ff133a0089d80 | did:kilt:4pnfkRn5UurBJTW92d9TaVLR2CqJdY4z5HPjrEbpGyBykare | did:kilt:4pehddkhEanexVTTzWAtrrfo2R7xPnePpuiJLC7shQU894aY | Twitch |
| Telegram | 0xcef8f3fe5aa7379faea95327942fd77287e1c144e3f53243e55705f11e890a4c | did:kilt:4pnfkRn5UurBJTW92d9TaVLR2CqJdY4z5HPjrEbpGyBykare | did:kilt:4pehddkhEanexVTTzWAtrrfo2R7xPnePpuiJLC7shQU894aY | Telegram |
| Youtube | 0x329a2a5861ea63c250763e5e4c4d4a18fe4470a31e541365c7fb831e5432b940 | did:kilt:4pnfkRn5UurBJTW92d9TaVLR2CqJdY4z5HPjrEbpGyBykare | did:kilt:4pehddkhEanexVTTzWAtrrfo2R7xPnePpuiJLC7shQU894aY | Youtube |
rompemoldes marked this conversation as resolved.
Show resolved Hide resolved

Additionally, for access to a broader range of CType Hashes, consider visiting the [CType Hub](https://ctypehub.galaniprojects.de/).

### Setup your environment

Expand Down
16 changes: 8 additions & 8 deletions backend/src/access/login.ts
View file
Open in desktop
aybarsayan marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import { Response, Request } from 'express'

import { emailRequest } from '../credentials/listOfRequests'
import { cTypeToRequest } from '../credentials/ctypeRequest'
import { buildCredentialRequest } from '../credentials/buildCredentialRequest'
import { verifySubmittedCredential } from '../credentials/verifySubmittedCredential'

import { setAccessCookie } from './setAccessCookie'

// Here you can set which type of credential (cType) your dApp will request users to login.
// You can change it by importing a different one from the list.
const requestedCTypeForLogin = emailRequest

/** First half of the login with credentials.*/
export async function buildLoginCredentialRequest(
Expand All @@ -19,7 +16,7 @@ export async function buildLoginCredentialRequest(
const encryptedCredentialRequest = await buildCredentialRequest(
request,
response,
requestedCTypeForLogin
cTypeToRequest
aybarsayan marked this conversation as resolved.
Show resolved Hide resolved
)
// With this, the extension will know what kind of credential to share
response.status(200).send(encryptedCredentialRequest)
Expand All @@ -37,13 +34,16 @@ export async function handleLoginCredentialSubmission(
const verifiedCredential = await verifySubmittedCredential(
request,
response,
requestedCTypeForLogin
cTypeToRequest
)

// Send a little something to the frontend, so that the user interface can display who logged in.
// The frontend can't read the encrypted credential; only the backend has the key to decrypt it.
// "Email" is capitalized on this cType schema
const plainUserInfo = verifiedCredential.claim.contents.Email
const claimContents = verifiedCredential.claim.contents;
// Check if any properties have been provided. If not, log in as 'Anonymous User'.
// If any property exists, send the object's first value as 'authenticationToken'
// to ensure login with any 'ctype'.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, authenticationToken does not have the plain user info, but rather something only the server can identify. How authentification tokens are made changes from website to website. Because we did not want to prescribe people how to do it, we took the simplest approach (some plain user info) and encourage people customize it afterwards.

Suggested change
// Check if any properties have been provided. If not, log in as 'Anonymous User'.
// If any property exists, send the object's first value as 'authenticationToken'
// to ensure login with any 'ctype'.
// Check if any properties have been provided. If not, send 'Anonymous User' to display on the frontend.
// If any property exists, send object's first attribute value,
// ensuring compatibility with any 'cType'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done at: 4fe1b4e

const plainUserInfo = Object.keys(claimContents).length === 0 ? 'Anonymous User' : claimContents[Object.keys(claimContents)[0]];

console.log(
'Decrypted User Info that we are passing to the frontend:',
Expand Down
8 changes: 8 additions & 0 deletions backend/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ export const DAPP_DID_URI = loadEnv('DAPP_DID_URI') as Kilt.DidUri
export const DAPP_NAME = process.env.DAPP_NAME ?? 'Web3-Login-Demo'
export const JWT_SIGNER_SECRET = loadEnv('JWT_SIGNER_SECRET')


// Configurable Credential types
export const CTYPE_HASH = loadEnv("CTYPE_HASH")
export const TRUSTED_ATTESTERS =
aybarsayan marked this conversation as resolved.
Show resolved Hide resolved
loadEnv("TRUSTED_ATTESTERS")
export const REQUIRED_PROPERTIES =
loadEnv("REQUIRED_PROPERTIES")

export let DAPP_ACCOUNT_ADDRESS: string

function loadEnv(name: string) {
Expand Down
25 changes: 25 additions & 0 deletions backend/src/credentials/ctypeRequest.ts
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

This file does not include the cTypeRequest. The request is prepare on buildCredentialRequest.

Please, rename this file to cTypeToRequest.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please do this? 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

Now in plural cTypesToRequest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done at: 4868513

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as Kilt from '@kiltprotocol/sdk-js'

import { CTYPE_HASH, REQUIRED_PROPERTIES, TRUSTED_ATTESTERS } from '../config'

// Here you can set which type of credential (cType) your dApp will request users to login.
// You can change it by importing a different one.
// Establish which cTypes our dApp accepts and which attesters we trust:
Copy link
Contributor

Choose a reason for hiding this comment

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

I would replace this by adding an explainer (docs) to the variable cTypeToRequest.

Maybe something like this:

/** Establish which type of credential (cType) our dApp will request users to login,
 * which properties are necessary and which attesters we trust.
 *
 * This is defined by the constants imported from the config.
 * Modify the `env`-file to adapt it to your liking.
 */
export const cTypeToRequest: Kilt.IRequestCredentialContent = {

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add this and rename the variable to be in plural.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done at: 4868513


const trustedAttesters = TRUSTED_ATTESTERS.split(',').map((s)=> {
const trimmed = s.trim()
Kilt.Did.validateUri(trimmed)
return trimmed as Kilt.DidUri
})
const requiredProperties = REQUIRED_PROPERTIES.split(',').map((s)=> s.trim())
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use variable names that describe the variable.

Maybe didUri and property?



export const cTypeToRequest: Kilt.IRequestCredentialContent = {
cTypes: [
{
cTypeHash: CTYPE_HASH as `0x${string}`,
trustedAttesters,
requiredProperties
}
]
}
27 changes: 0 additions & 27 deletions backend/src/credentials/listOfRequests.ts
View file
Open in desktop

This file was deleted.