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

Initial implementation using Perun dumps and capabilities #2

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mesemus
Copy link
Contributor

@mesemus mesemus commented Sep 7, 2024

No description provided.

@mesemus mesemus requested a review from mirekys September 7, 2024 15:51
=======

CESNET OIDC Auth backend for OARepo

Copy link
Member

Choose a reason for hiding this comment

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

Forgot to add yourself to authors :)


4. Add the e-infra public key to your invenio.cfg or environment variables:
```python
EINFRA_RSA_KEY=b'-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAmho5h/lz6USUUazQaVT3\nPHloIk/Ljs2vZl/RAaitkXDx6aqpl1kGpS44eYJOaer4oWc6/QNaMtynvlSlnkuW\nrG765adNKT9sgAWSrPb81xkojsQabrSNv4nIOWUQi0Tjh0WxXQmbV+bMxkVaElhd\nHNFzUfHv+XqI8Hkc82mIGtyeMQn+VAuZbYkVXnjyCwwa9RmPOSH+O4N4epDXKk1V\nK9dUxf/rEYbjMNZGDva30do0mrBkU8W3O1mDVJSSgHn4ejKdGNYMm0JKPAgCWyPW\nJDoL092ctPCFlUMBBZ/OP3omvgnw0GaWZXxqSqaSvxFJkqCHqLMwpxmWTTAgEvAb\nnwIDAQAB\n-----END PUBLIC KEY-----\n'
Copy link
Member

Choose a reason for hiding this comment

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

From where this public key is obtained? (to update it if it ever changes)

Comment on lines +89 to +90
EINFRA_SYNC_SERVICE_ID = 0
"""Internal ID of the service in the E-INFRA Perun that is responsible for synchronization
Copy link
Member

Choose a reason for hiding this comment

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

What is the difference between SYNC_SERVICE and SERVICE here?

from marshmallow import ValidationError
from sqlalchemy import select

CommunityRole = namedtuple("CommunityRole", ["community_id", "role"])
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be part of oarepo-communities?

pass

@cached_property
def slug_to_id(self) -> Dict[str, str]:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be part of oarepo-communities? Nothing related to perun oidc

EINFRA_ENTITLEMENT_COMMUNITIES_GROUP_PARTS = [["cesnet.cz", "res", "communities"]]
"""Parts of the entitlement URN name that represent communities."""

EINFRA_DUMP_DATA_URL = "s3://einfra-dump-bucket"
Copy link
Member

Choose a reason for hiding this comment

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

How do we access that bucket? Which credentials are used?


This component is responsible for synchronizing the community to the E-INFRA Perun.
"""
communities_components = app.config.get("COMMUNITIES_SERVICE_COMPONENTS", None)
Copy link
Member

Choose a reason for hiding this comment

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

Check for EINFRA_COMMUNITY_SYNCHRONIZATION = True here? We shouldn't register this if that is disabled


# If there is no user identity for this user and group, create it
ui = UserIdentity.query.filter_by(
user=user, method="e-infra", id=decoded_token["sub"]
Copy link
Member

Choose a reason for hiding this comment

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

use remote.name for method?

user=user, method="e-infra", id=decoded_token["sub"]
).one_or_none()
if not ui:
UserIdentity.create(user, "e-infra", decoded_token["sub"])
Copy link
Member

Choose a reason for hiding this comment

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

use remote.name for method? to keep consistent with auccount_info_serializer

def autocreate_user(remote, token=None, response=None, account_info=None):
assert account_info is not None

email = account_info["user"]["email"].lower()
Copy link
Member

Choose a reason for hiding this comment

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

Why is e-mail lowercase trasnformation needed?

The caller must have the permission to upload the dump (upload-oidc-einfra-dump action
that can be assigned via invenio access commandline tool).
"""
if not Permission(upload_dump_action).allows(g.identity):
Copy link
Member

Choose a reason for hiding this comment

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

Consider stronger verification (cert-based? whitelist, signature checking) of the request

return {
"status": "error",
"message": "Content-Type must be application/json",
}, 400
Copy link
Member

Choose a reason for hiding this comment

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

This should be http 415 unsupportedMediaType error

route("POST", routes["upload-dump"], self.upload_dump),
]

def upload_dump(self):
Copy link
Member

Choose a reason for hiding this comment

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

Configure rate-limit for this endpoint to prevent flooding of target location with dumps in case of a mistake or bad intention on requestor side

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.

2 participants