Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Group membership check #1074

Merged
merged 136 commits into from
Nov 22, 2021
Merged
Show file tree
Hide file tree
Changes from 130 commits
Commits
Show all changes
136 commits
Select commit Hold shift + click to select a range
bd8d425
migrate to msgraph
chkeita Jun 7, 2021
1ad0f5a
add subscription id to query_microsoft_graph
chkeita Jun 7, 2021
ac0f400
migrating remaingin references
chkeita Jun 8, 2021
4902528
formatting
chkeita Jun 8, 2021
a4379c8
adding missing dependencies
chkeita Jun 8, 2021
098ac3c
Merge branch 'main' into msgraph
chkeita Jun 9, 2021
e1fa0ad
flake fix
chkeita Jun 9, 2021
c130d6d
fix get_tenant_id
chkeita Jun 9, 2021
b6bf69f
Merge branch 'main' into msgraph
chkeita Jun 9, 2021
681867e
cleanup
chkeita Jun 10, 2021
8230024
formatting
chkeita Jun 14, 2021
1a60c07
Merge remote-tracking branch 'upstream/main' into msgraph
chkeita Jun 14, 2021
dd989fb
Merge remote-tracking branch 'upstream/main' into msgraph
chkeita Jun 16, 2021
0403485
migrate application creation in deploy.py
chkeita Jun 16, 2021
6245563
foramt
chkeita Jun 16, 2021
431200d
mypy fix
chkeita Jun 16, 2021
71bceef
isort
chkeita Jun 16, 2021
7e1b93c
isort
chkeita Jun 16, 2021
7ba55de
format
chkeita Jun 16, 2021
09107ab
bug fixes
chkeita Jun 17, 2021
73a135f
specify the correct signInAudience
chkeita Jun 17, 2021
be18ab9
fix backing service principal creation
chkeita Jun 17, 2021
295c56e
remove remaining references to graphrbac
chkeita Jun 17, 2021
9c47159
fix ms graph authentication
chkeita Jun 18, 2021
99df719
Merge branch 'main' into msgraph
chkeita Jun 30, 2021
b9915c1
Merge branch 'main' into msgraph
chkeita Jul 9, 2021
128c7f8
formatting
chkeita Jul 9, 2021
d5ec3cc
Merge branch 'main' into msgraph
chkeita Jul 15, 2021
83a2904
add a data structure to store permission for each url
chkeita Jul 15, 2021
f536f77
adding logic
chkeita Jul 16, 2021
e1a4918
reafctoring and added a couple of tests
chkeita Jul 16, 2021
57488b0
foramtting
chkeita Jul 16, 2021
208b822
Merge remote-tracking branch 'upstream/main' into msgraph
chkeita Jul 16, 2021
12c205f
Merge remote-tracking branch 'upstream/main' into chkeita/group_check
chkeita Jul 16, 2021
f8b48a3
more unit tests
chkeita Jul 19, 2021
e8be5fa
build fix
chkeita Jul 19, 2021
de717c0
format
chkeita Jul 19, 2021
ab14727
fix
chkeita Jul 19, 2021
8952156
formatting
chkeita Jul 19, 2021
0e53540
adding testing function
chkeita Jul 19, 2021
6bb50cd
formatting
chkeita Jul 19, 2021
a70fbd6
formatting
chkeita Jul 19, 2021
a9e0bf8
build fix
chkeita Jul 19, 2021
e4618f8
bug fixes
chkeita Jul 20, 2021
4d6bcd9
add support for http method
chkeita Jul 20, 2021
d995aab
Merge remote-tracking branch 'upstream/main' into msgraph
chkeita Jul 20, 2021
b58ac55
fix tests
chkeita Jul 20, 2021
8fbead2
Merge remote-tracking branch 'upstream/main' into chkeita/group_check
chkeita Jul 20, 2021
31e5341
build fix attempt
chkeita Jul 20, 2021
c3e5b38
fix typo
chkeita Jul 20, 2021
174cd86
format
chkeita Jul 20, 2021
088dc4d
deployment fix
chkeita Jul 20, 2021
10351cb
Merge branch 'msgraph' into chkeita/group_check
chkeita Jul 20, 2021
b402038
set implicitGrantSettings in the deployment
chkeita Jul 21, 2021
9996678
format
chkeita Jul 21, 2021
b9fcdac
cleanup
chkeita Jul 21, 2021
f52c757
format
chkeita Jul 21, 2021
212f18a
renaming RequestAuthorization to RequestAccess
chkeita Jul 21, 2021
37f2cab
Merge branch 'main' into msgraph
chkeita Jul 21, 2021
f8c4951
Merge branch 'main' into chkeita/group_check
chkeita Jul 22, 2021
998829d
fix deployment
chkeita Jul 27, 2021
63cde94
fix graph authentication on the server
chkeita Jul 27, 2021
93ee8c9
use the current cli logged in account to retrive the backend token cache
chkeita Jul 29, 2021
e937286
assign the the msgraph app role permissions to the web app during the…
chkeita Jul 29, 2021
642c37b
formatting
chkeita Jul 29, 2021
05ba100
Merge remote-tracking branch 'upstream/main' into msgraph
chkeita Jul 29, 2021
398af21
fix build
chkeita Jul 29, 2021
b58a04d
build fix
chkeita Jul 29, 2021
c490239
fix bandit issue
chkeita Jul 29, 2021
79b050d
mypy fix
chkeita Jul 29, 2021
92ad8e9
isort
chkeita Jul 29, 2021
4f9b98f
deploy fixes
chkeita Jul 30, 2021
66d3bed
formatting
chkeita Jul 30, 2021
4c805d7
Merge remote-tracking branch 'upstream/main' into msgraph
chkeita Aug 2, 2021
c6b2c31
Merge branch 'msgraph' into chkeita/group_check
chkeita Aug 2, 2021
c15fb28
removing assign_app_permissions from the deployment becahse it requir…
chkeita Aug 3, 2021
161d092
remove assign_app_permissions
chkeita Aug 4, 2021
96bde11
Merge remote-tracking branch 'upstream/main' into chkeita/group_check
chkeita Aug 4, 2021
7ca5d9b
remove assign_app_permissions
chkeita Aug 6, 2021
eb7b574
Merge remote-tracking branch 'upstream/main' into msgraph
chkeita Aug 6, 2021
2573f79
Merge branch 'msgraph' into chkeita/group_check
chkeita Aug 6, 2021
0b11e2e
Merge branch 'main' into msgraph
chkeita Aug 23, 2021
074c85a
Merge branch 'main' into msgraph
chkeita Aug 30, 2021
e4adb97
mypy fix
chkeita Aug 30, 2021
5160276
Merge branch 'msgraph' into chkeita/group_check
chkeita Aug 30, 2021
77de4c2
Merge branch 'main' into chkeita/group_check
chkeita Aug 30, 2021
035a55f
Merge branch 'main' into chkeita/group_check
chkeita Oct 25, 2021
c2842a4
format
chkeita Oct 25, 2021
a7a442c
more cleanup
chkeita Oct 25, 2021
2959c77
build fix
chkeita Oct 26, 2021
fa3086a
remove change to azuredeploy.json
chkeita Oct 26, 2021
c361c6e
lint
chkeita Oct 26, 2021
f42b51b
Merge remote-tracking branch 'upstream/main' into chkeita/group_check
chkeita Oct 29, 2021
3a4fdb5
fix mypy
chkeita Oct 29, 2021
945affe
adding GroupMembershipChecker abstraction
chkeita Oct 30, 2021
a7e3940
moving apiaccessrules to instance config
chkeita Oct 30, 2021
0337a0e
moving to github 3.8 to use protocols
chkeita Oct 30, 2021
5585d8c
moving group_membership into its own file
chkeita Nov 1, 2021
dd455b8
refactoring
chkeita Nov 1, 2021
04ccc18
Merge branch 'main' into chkeita/group_check
chkeita Nov 1, 2021
45d99f7
generate docs
chkeita Nov 2, 2021
14cbb67
Revert "moving to github 3.8 to use protocols"
chkeita Nov 2, 2021
e42d06a
move service to 3.8
chkeita Nov 2, 2021
545b61b
Merge branch 'main' into chkeita/group_check
chkeita Nov 2, 2021
af5c3e9
Merge remote-tracking branch 'upstream/main' into chkeita/group_check
chkeita Nov 5, 2021
eeb2d13
adding functional tests
chkeita Nov 8, 2021
45aea08
format
chkeita Nov 8, 2021
438435c
generate docs
chkeita Nov 8, 2021
21fc030
Merge branch 'main' into chkeita/group_check
chkeita Nov 8, 2021
dac2570
run tests only in the test folder
chkeita Nov 8, 2021
d948322
build fix
chkeita Nov 8, 2021
d75f3f5
cleanup
chkeita Nov 8, 2021
b1de7c0
build fix
chkeita Nov 9, 2021
e047646
foramtting
chkeita Nov 9, 2021
745f40f
fix functional tests
chkeita Nov 9, 2021
804026f
flake fix
chkeita Nov 9, 2021
5e4434d
lint
chkeita Nov 9, 2021
1cba246
- return None when not rule is found
chkeita Nov 15, 2021
670dc9c
fix type in tests
chkeita Nov 16, 2021
c1023a2
fix test
chkeita Nov 16, 2021
b475c1f
better error message
chkeita Nov 16, 2021
51e84cf
format
chkeita Nov 16, 2021
735e7b1
remove old implementation of check_access
chkeita Nov 16, 2021
4996949
Merge branch 'main' into chkeita/group_check
chkeita Nov 16, 2021
130891a
fix typo is name
chkeita Nov 17, 2021
0ad5d0f
generate docs
chkeita Nov 17, 2021
00c4358
format
chkeita Nov 17, 2021
93e1236
format
chkeita Nov 17, 2021
5cf3729
mypy fix
chkeita Nov 17, 2021
d46fa1a
Merge branch 'main' into chkeita/group_check
chkeita Nov 17, 2021
5f73e99
add unit tests to StaticGroupMembership
chkeita Nov 18, 2021
5c46c7d
adding comments
chkeita Nov 19, 2021
abb2a90
removing NO_REQUEST_ACCESS_RULES_CACHE
chkeita Nov 22, 2021
882ebd4
format
chkeita Nov 22, 2021
41d9c24
more formatting
chkeita Nov 22, 2021
5aac3ef
Merge branch 'main' into chkeita/group_check
chkeita Nov 22, 2021
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ jobs:
pip install ${GITHUB_WORKSPACE}/artifacts/sdk/onefuzztypes-*.whl
pip install -r __app__/requirements.txt
pip install -r requirements-dev.txt
pytest
pytest tests
flake8 .
bandit -r ./__app__/
black ./__app__/ ./tests --check
Expand Down
86 changes: 86 additions & 0 deletions docs/webhook_events.md
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,31 @@ Each event will be submitted via HTTP POST to the user provided URL.
```json
{
"definitions": {
"ApiAccessRule": {
"properties": {
"allowed_groups": {
"items": {
"format": "uuid",
"type": "string"
},
"title": "Allowed Groups",
"type": "array"
},
"methods": {
"items": {
"type": "string"
},
"title": "Methods",
"type": "array"
}
},
"required": [
"methods",
"allowed_groups"
],
"title": "ApiAccessRule",
"type": "object"
},
"AzureMonitorExtensionConfig": {
"properties": {
"config_version": {
Expand Down Expand Up @@ -757,9 +782,27 @@ Each event will be submitted via HTTP POST to the user provided URL.
"title": "Allowed Aad Tenants",
"type": "array"
},
"api_access_rules": {
"additionalProperties": {
"$ref": "#/definitions/ApiAccessRule"
},
"title": "Api Access Rules",
"type": "object"
},
"extensions": {
"$ref": "#/definitions/AzureVmExtensionConfig"
},
"group_membership": {
"additionalProperties": {
"items": {
"format": "uuid",
"type": "string"
},
"type": "array"
},
"title": "Group Membership",
"type": "object"
},
"network_config": {
"$ref": "#/definitions/NetworkConfig"
},
Expand Down Expand Up @@ -4933,6 +4976,31 @@ Each event will be submitted via HTTP POST to the user provided URL.
```json
{
"definitions": {
"ApiAccessRule": {
"properties": {
"allowed_groups": {
"items": {
"format": "uuid",
"type": "string"
},
"title": "Allowed Groups",
"type": "array"
},
"methods": {
"items": {
"type": "string"
},
"title": "Methods",
"type": "array"
}
},
"required": [
"methods",
"allowed_groups"
],
"title": "ApiAccessRule",
"type": "object"
},
"Architecture": {
"description": "An enumeration.",
"enum": [
Expand Down Expand Up @@ -5856,9 +5924,27 @@ Each event will be submitted via HTTP POST to the user provided URL.
"title": "Allowed Aad Tenants",
"type": "array"
},
"api_access_rules": {
"additionalProperties": {
"$ref": "#/definitions/ApiAccessRule"
},
"title": "Api Access Rules",
"type": "object"
},
"extensions": {
"$ref": "#/definitions/AzureVmExtensionConfig"
},
"group_membership": {
"additionalProperties": {
"items": {
"format": "uuid",
"type": "string"
},
"type": "array"
},
"title": "Group Membership",
"type": "object"
},
"network_config": {
"$ref": "#/definitions/NetworkConfig"
},
Expand Down
8 changes: 0 additions & 8 deletions src/api-service/__app__/onefuzzlib/azure/creds.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,6 @@ def query_microsoft_graph_list(
raise GraphQueryError("Expected data containing a list of values", None)


def is_member_of(group_id: str, member_id: str) -> bool:
body = {"groupIds": [group_id]}
response = query_microsoft_graph_list(
method="POST", resource=f"users/{member_id}/checkMemberGroups", body=body
)
return group_id in response


@cached
def get_scaleset_identity_resource_path() -> str:
scaleset_id_name = "%s-scalesetid" % get_instance_name()
Expand Down
51 changes: 51 additions & 0 deletions src/api-service/__app__/onefuzzlib/azure/group_membership.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#!/usr/bin/env python
#
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.

from typing import Dict, List, Protocol
from uuid import UUID

from ..config import InstanceConfig
from .creds import query_microsoft_graph_list


class GroupMembershipChecker(Protocol):
def is_member(self, group_ids: List[UUID], member_id: UUID) -> bool:
"""Check if member is part of at least one of the groups"""
if member_id in group_ids:
return True

groups = self.get_groups(member_id)
for g in group_ids:
if g in groups:
return True

return False

def get_groups(self, member_id: UUID) -> List[UUID]:
"""Gets all the groups of the provided member"""
stishkin marked this conversation as resolved.
Show resolved Hide resolved


def create_group_membership_checker() -> GroupMembershipChecker:
config = InstanceConfig.fetch()
if config.group_membership:
return StaticGroupMembership(config.group_membership)
else:
return AzureADGroupMembership()


class AzureADGroupMembership(GroupMembershipChecker):
def get_groups(self, member_id: UUID) -> List[UUID]:
response = query_microsoft_graph_list(
method="GET", resource=f"users/{member_id}/transitiveMemberOf"
)
return response


class StaticGroupMembership(GroupMembershipChecker):
def __init__(self, memberships: Dict[str, List[UUID]]):
self.memberships = memberships

def get_groups(self, member_id: UUID) -> List[UUID]:
return self.memberships.get(str(member_id), [])
61 changes: 53 additions & 8 deletions src/api-service/__app__/onefuzzlib/request.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,81 @@
import json
import logging
import os
import urllib
from typing import TYPE_CHECKING, Optional, Sequence, Type, TypeVar, Union
from uuid import UUID

from azure.functions import HttpRequest, HttpResponse
from memoization import cached
from onefuzztypes.enums import ErrorCode
from onefuzztypes.models import Error
from onefuzztypes.responses import BaseResponse
from pydantic import BaseModel # noqa: F401
from pydantic import ValidationError

from .azure.group_membership import create_group_membership_checker
from .config import InstanceConfig
from .orm import ModelMixin
from .request_access import RequestAccess

# We don't actually use these types at runtime at this time. Rather,
# these are used in a bound TypeVar. MyPy suggests to only import these
# types during type checking.
if TYPE_CHECKING:
from onefuzztypes.requests import BaseRequest # noqa: F401
from pydantic import BaseModel # noqa: F401


def get_rules() -> Optional[RequestAccess]:
config = InstanceConfig.fetch()
if config.api_access_rules:
return RequestAccess.build(config.api_access_rules)
else:
return None


@cached(ttl=60)
def get_rules_cached() -> Optional[RequestAccess]:
return get_rules()


def check_access(req: HttpRequest) -> Optional[Error]:
chkeita marked this conversation as resolved.
Show resolved Hide resolved
if "ONEFUZZ_AAD_GROUP_ID" in os.environ:
message = "ONEFUZZ_AAD_GROUP_ID configuration not supported"
logging.error(message)
return Error(
code=ErrorCode.INVALID_CONFIGURATION,
errors=[message],
)
if "NO_REQUEST_ACCESS_RULES_CACHE" in os.environ:
stishkin marked this conversation as resolved.
Show resolved Hide resolved
rules = get_rules()
else:
rules = get_rules_cached()

if not rules:
stishkin marked this conversation as resolved.
Show resolved Hide resolved
return None

path = urllib.parse.urlparse(req.url).path
rule = rules.get_matching_rules(req.method, path)

if not rule:
return None

member_id = UUID(req.headers["x-ms-client-principal-id"])

try:
membership_checker = create_group_membership_checker()
result = membership_checker.is_member(rule.allowed_groups_ids, member_id)
if not result:
logging.error(
"unauthorized access: %s is not authorized to access in %s",
member_id,
req.url,
)
return Error(
code=ErrorCode.UNAUTHORIZED,
errors=["not approved to use this endpoint"],
)
except Exception as e:
return Error(
code=ErrorCode.UNAUTHORIZED,
errors=["unable to interact with graph", str(e)],
)

return None


def ok(
data: Union[BaseResponse, Sequence[BaseResponse], ModelMixin, Sequence[ModelMixin]]
Expand Down
24 changes: 9 additions & 15 deletions src/api-service/__app__/onefuzzlib/request_access.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
from typing import Dict, List
from typing import Dict, List, Optional
from uuid import UUID

from onefuzztypes.models import ApiAccessRule
from pydantic import parse_raw_as


class RuleConflictError(Exception):
Expand Down Expand Up @@ -41,7 +40,7 @@ def __init__(self) -> None:
def __add_url__(self, methods: List[str], path: str, rules: Rules) -> None:
methods = list(map(lambda m: m.upper(), methods))

segments = path.split("/")
segments = [s for s in path.split("/") if s != ""]
if len(segments) == 0:
return

Expand Down Expand Up @@ -71,15 +70,14 @@ def __add_url__(self, methods: List[str], path: str, rules: Rules) -> None:
for method in methods:
current_node.rules[method] = rules

def get_matching_rules(self, method: str, path: str) -> Rules:
def get_matching_rules(self, method: str, path: str) -> Optional[Rules]:
method = method.upper()
segments = path.split("/")
segments = [s for s in path.split("/") if s != ""]
current_node = self.root
current_rule = None

if method in current_node.rules:
current_rule = current_node.rules[method]
else:
current_rule = RequestAccess.Rules()

current_segment_index = 0

Expand All @@ -98,17 +96,13 @@ def get_matching_rules(self, method: str, path: str) -> Rules:
return current_rule

@classmethod
def parse_rules(cls, rules_data: str) -> "RequestAccess":
rules = parse_raw_as(List[ApiAccessRule], rules_data)
return cls.build(rules)

@classmethod
def build(cls, rules: List[ApiAccessRule]) -> "RequestAccess":
def build(cls, rules: Dict[str, ApiAccessRule]) -> "RequestAccess":
request_access = RequestAccess()
for rule in rules:
for endpoint in rules:
rule = rules[endpoint]
request_access.__add_url__(
rule.methods,
rule.endpoint,
endpoint,
RequestAccess.Rules(allowed_groups_ids=rule.allowed_groups),
)

Expand Down
Loading