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

start refactoring creds behaviour for address #849

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
12 changes: 11 additions & 1 deletion src/@types/DDO/Credentials.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
// we will have more (user defined)
export const KNOWN_CREDENTIALS_TYPES = ['address', 'accessList']
export const KNOWN_CREDENTIALS_TYPES = ['address', 'accessList'] // the ones we handle

export const CREDENTIAL_TYPES = {
ADDRESS: KNOWN_CREDENTIALS_TYPES[0],
ACCESS_LIST: KNOWN_CREDENTIALS_TYPES[1],
POLICY_SERVER_SPECIFIC: 'PS-specific Type' // externally handled by Policy Server
}
export interface Credential {
type?: string
values?: string[]
}

export type MATCH_RULES = 'any' | 'all'

export interface Credentials {
match_allow?: MATCH_RULES // any => it's enough to have one rule matched, all => all allow rules should match, default: 'all'
match_deny?: MATCH_RULES // same pattern as above, default is 'any'
allow?: Credential[]
deny?: Credential[]
}
4 changes: 2 additions & 2 deletions src/components/core/handler/downloadHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ export class DownloadHandler extends Handler {
).success
} else {
accessGrantedDDOLevel = areKnownCredentialTypes(ddo.credentials)
? checkCredentials(ddo.credentials, task.consumerAddress)
? checkCredentials(ddo.credentials, task.consumerAddress, ddo.chainId)
: true
}
if (!accessGrantedDDOLevel) {
Expand Down Expand Up @@ -397,7 +397,7 @@ export class DownloadHandler extends Handler {
).success)
} else {
accessGrantedServiceLevel = areKnownCredentialTypes(service.credentials)
? checkCredentials(service.credentials, task.consumerAddress)
? checkCredentials(service.credentials, task.consumerAddress, chainId)
: true
}

Expand Down
14 changes: 13 additions & 1 deletion src/test/data/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ const nftLevelCredentials: Credentials = {
},
{
type: 'address',
values: ['0xA78deb2Fa79463945C247991075E2a0e98Ba7A09']
values: [
'0xA78deb2Fa79463945C247991075E2a0e98Ba7A09',
'0xe2DD09d719Da89e5a3D0F2549c7E24566e947260'
]
}
],
deny: [
Expand All @@ -84,6 +87,15 @@ const serviceLevelCredentials: Credentials = {
type: 'address',
values: ['0xA78deb2Fa79463945C247991075E2a0e98Ba7A09']
}
],
allow: [
{
type: 'address',
values: [
'0xe2DD09d719Da89e5a3D0F2549c7E24566e947260',
'0xBE5449a6A97aD46c8558A3356267Ee5D2731ab5e'
]
}
]
}

Expand Down
50 changes: 38 additions & 12 deletions src/test/unit/credentials.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from '../utils/utils.js'
import { ENVIRONMENT_VARIABLES } from '../../utils/constants.js'
import { homedir } from 'os'
import { DEVELOPMENT_CHAIN_ID } from '../../utils/address.js'

let envOverrides: OverrideEnvConfig[]

Expand All @@ -28,25 +29,26 @@ describe('credentials', () => {
envOverrides = await setupEnvironment(null, envOverrides)
})

it('should allow access with undefined or empty credentials', () => {
it('should deny access with undefined or empty credentials', () => {
const credentialsUndefined: Credentials = undefined
const consumerAddress = '0x123'
const accessGranted1 = checkCredentials(credentialsUndefined, consumerAddress)
expect(accessGranted1).to.equal(true)
expect(accessGranted1).to.equal(false)
const credentialsEmapty = {} as Credentials
const accessGranted2 = checkCredentials(credentialsEmapty, consumerAddress)
expect(accessGranted2).to.equal(true)
expect(accessGranted2).to.equal(false)
})
it('should allow access with empty allow and deny lists', () => {
it('should deny access with empty allow and deny lists', () => {
// if list does not exist or is empty access is denied
const credentials: Credentials = {
allow: [],
deny: []
}
const consumerAddress = '0x123'
const accessGranted = checkCredentials(credentials, consumerAddress)
expect(accessGranted).to.equal(true)
expect(accessGranted).to.equal(false)
})
it('should allow access with empty values in deny lists', () => {
it('should deny access with empty values in deny lists', () => {
const credentials: Credentials = {
deny: [
{
Expand All @@ -57,22 +59,22 @@ describe('credentials', () => {
}
const consumerAddress = '0x123'
const accessGranted = checkCredentials(credentials, consumerAddress)
expect(accessGranted).to.equal(true)
expect(accessGranted).to.equal(false)
})

it('should allow access with "accessList" credentials type', () => {
it('should deny access with "accessList" credentials (default behaviour if cannot check)', () => {
const consumerAddress = '0x123'
const credentials: Credentials = {
deny: [
{
type: 'accessList',
values: [consumerAddress]
values: [consumerAddress] // not a valid SC address anyway
}
]
}

const accessGranted = checkCredentials(credentials, consumerAddress)
expect(accessGranted).to.equal(true)
expect(accessGranted).to.equal(false)
})

it('should deny access with empty values in allow lists', () => {
Expand Down Expand Up @@ -101,7 +103,7 @@ describe('credentials', () => {
const accessGranted = checkCredentials(credentials, consumerAddress)
expect(accessGranted).to.equal(true)
})
it('should allow access with address not in deny list', () => {
it('should deny access with address not explicitly in deny list but also without any allow list', () => {
const credentials: Credentials = {
deny: [
{
Expand All @@ -112,7 +114,7 @@ describe('credentials', () => {
}
const consumerAddress = '0x123'
const accessGranted = checkCredentials(credentials, consumerAddress)
expect(accessGranted).to.equal(true)
expect(accessGranted).to.equal(false) // its not denied explicitly but not allowed either
})
it('should deny access with address in deny list', () => {
const credentials: Credentials = {
Expand Down Expand Up @@ -227,6 +229,30 @@ describe('credentials', () => {
expect(hasAddressMatchAllRule(creds2.credentials.allow)).to.be.equal(false)
})

it('should deny access by default if no specific allow rule is a match', () => {
const credentials: Credentials = {
allow: [
{
type: 'address',
values: []
}
],
deny: [
{
type: 'address',
values: []
}
]
}
const consumerAddress = '0x123'
const accessGranted = checkCredentials(
credentials,
consumerAddress,
DEVELOPMENT_CHAIN_ID
)
expect(accessGranted).to.equal(false)
})

after(async () => {
await tearDownEnvironment(envOverrides)
})
Expand Down
88 changes: 66 additions & 22 deletions src/utils/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ethers, Signer } from 'ethers'
import { CORE_LOGGER } from './logging/common.js'
import {
Credential,
CREDENTIAL_TYPES,
Credentials,
KNOWN_CREDENTIALS_TYPES
} from '../@types/DDO/Credentials.js'
Expand All @@ -13,7 +14,7 @@ import { isDefined } from './util.js'
export function findCredential(
credentials: Credential[],
consumerCredentials: Credential
) {
): Credential | undefined {
return credentials.find((credential) => {
if (Array.isArray(credential?.values)) {
if (credential.values.length > 0) {
Expand All @@ -29,18 +30,38 @@ export function findCredential(
})
}

export function isAddressCredentialMatch(
credential: Credential,
consumerCredentials: Credential
): boolean {
if (credential?.type?.toLowerCase() !== CREDENTIAL_TYPES.ADDRESS) {
return false
}
if (credential.values.length > 0) {
const credentialValues = credential.values.map((v) => String(v)?.toLowerCase())
return credentialValues.includes(consumerCredentials.values[0])
}

return false
}

function isAddressMatchAll(credential: Credential): boolean {
if (credential?.type?.toLowerCase() !== CREDENTIAL_TYPES.ADDRESS) {
return false
}
if (credential.values.length > 0) {
const filteredValues: string[] = credential.values.filter((value: string) => {
return value?.toLowerCase() === '*' // address
})
return filteredValues.length > 0
}
return false
}

export function hasAddressMatchAllRule(credentials: Credential[]): boolean {
const creds = credentials.find((credential: Credential) => {
if (Array.isArray(credential?.values)) {
if (credential.values.length > 0 && credential.type) {
const filteredValues: string[] = credential.values.filter((value: string) => {
return value?.toLowerCase() === '*' // address
})
return (
filteredValues.length > 0 &&
credential.type.toLowerCase() === KNOWN_CREDENTIALS_TYPES[0]
)
}
return isAddressMatchAll(credential)
}
return false
})
Expand All @@ -52,29 +73,52 @@ export function hasAddressMatchAllRule(credentials: Credential[]): boolean {
* @param credentials credentials
* @param consumerAddress consumer address
*/
export function checkCredentials(credentials: Credentials, consumerAddress: string) {
export function checkCredentials(
credentials: Credentials,
consumerAddress: string,
chainId?: number
) {
const consumerCredentials: Credential = {
type: 'address',
type: CREDENTIAL_TYPES.ADDRESS, // 'address',
values: [String(consumerAddress)?.toLowerCase()]
}

const accessGranted = true
const accessGranted = false
// check deny access
// https://github.com/oceanprotocol/ocean-node/issues/810
// for deny rules: if value does not exist or it's empty -> there is no deny list. if value list has at least one element, check it
if (Array.isArray(credentials?.deny) && credentials.deny.length > 0) {
const accessDeny = findCredential(credentials.deny, consumerCredentials)
// credential is on deny list, so it should be blocked access
if (accessDeny) {
return false
for (const cred of credentials.deny) {
const { type } = cred
if (type === CREDENTIAL_TYPES.ADDRESS) {
const accessDeny = isAddressCredentialMatch(cred, consumerCredentials)
// credential is on deny list, so it should be blocked access
if (accessDeny) {
return false
}
// credential not found, so it really depends if we have a match on the allow list instead
}
// else TODO later
// support also for access list type here
// https://github.com/oceanprotocol/ocean-node/issues/840
// else if (type === CREDENTIAL_TYPES.ACCESS_LIST && chainId) {
// }
}
// credential not found, so it really depends if we have a match
}
// check allow access
// for allow rules: if value does not exist or it's empty -> no one has access. if value list has at least one element, check it
if (Array.isArray(credentials?.allow) && credentials.allow.length > 0) {
const accessAllow = findCredential(credentials.allow, consumerCredentials)
if (accessAllow || hasAddressMatchAllRule(credentials.allow)) {
return true
for (const cred of credentials.allow) {
const { type } = cred
if (type === CREDENTIAL_TYPES.ADDRESS) {
const accessAllow = isAddressCredentialMatch(cred, consumerCredentials)
if (accessAllow || isAddressMatchAll(cred)) {
return true
}
}
// else if (type === CREDENTIAL_TYPES.ACCESS_LIST && chainId) {
// }
}
return false
}
return accessGranted
}
Expand Down
Loading