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

Jump to bottom

[SDESK-7484] Create async Content API Subscriber Token and Auth #2825

Open
wants to merge 17 commits into
base: async
Choose a base branch
from
Open

[SDESK-7484] Create async Content API Subscriber Token and Auth #2825

wants to merge 17 commits into from

Conversation

eos87
Copy link

@eos87 eos87 commented Feb 3, 2025 •
edited
Loading

Purpose

Implement a new token-based authentication system specific to the ContentAPI, replacing the default Superdesk async auth with a custom SubscriberToken mechanism.

What has changed

  • Introduced a new SubscriberToken resource model and an accompanying async service.
  • Created a ResourceConfig for the SubscriberToken resource, enabling its REST API. Added corresponding module.
  • Implemented a new SubscriberTokenAuth class for the ContentAPI.
  • Set the ASYNC_AUTH_CLASS config in the ContentAPI app to the new auth class.
  • Removed the old SubscriberTokenAuth class and related resource code.
  • Updated necessary imports and configurations to reflect the new changes.
  • Implemented optionally generating nested HATEOAS information for items in async rest api.
  • Allow deleting items via rest API by fixing issue with CORS and wrong methods (it was using resource_methods instead of items_methods).

SDESK-7484

@eos87 eos87 changed the title Create new async Content API Subscriber Token Resource and Auth [SDESK-7484] Create new async Content API Subscriber Token Resource and Auth Feb 3, 2025
@eos87 eos87 changed the title [SDESK-7484] Create new async Content API Subscriber Token Resource and Auth [SDESK-7484] Create async Content API Subscriber Token and Auth Feb 4, 2025
@eos87 eos87 removed the WIP label Feb 4, 2025
@eos87 eos87 marked this pull request as ready for review February 4, 2025 16:30
@eos87 eos87 requested a review from MarkLark86 February 4, 2025 16:31
MarkLark86
MarkLark86 reviewed Feb 5, 2025
View reviewed changes
Copy link
Contributor

@MarkLark86 MarkLark86 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. A minor improvement regarding HATEOAS & related items.

As for the tests, there are some failing, and it would be very beneficial if we had tests for the item-level HATEOAS, especially considering it's a required feature for some front-end features to work.

superdesk/core/resources/resource_rest_endpoints.py Outdated Show resolved Hide resolved
eos87 added 6 commits February 5, 2025 14:17
@eos87
Move related links generation to ResourceModel
31ed419 
Move related links generation from REST endpoints to ResourceModel for better encapsulation and reusability.
Cache relationships at class creation time to avoid potential performance issues because of the extraction of annotations.
Add utilities for consistent URL generation and annotation handling across class hierarchies.
@eos87
Fix mypy complains. Some type errors ignored
d40b884 
Added some TODO-ASYNC comments to some ignored type errors for now
@eos87
Fix flake8 complains
9d6be3a 
SDESK-7484
@eos87
Split resource_endpoints_test.py as it is getting too big
f321cc9 
SDESK-7484
@eos87
Fix broken tests
0990545 
SDESK-7484
@eos87
Rename from nose-tests.yml to tests.yml simply
bd8fcc4 
We no longer use `nose` so there is not reason to have the file name with it
@eos87 eos87 added this to the 3.0 milestone Feb 5, 2025
@eos87
Copy link
Author

eos87 commented Feb 5, 2025

As for the tests, there are some failing, and it would be very beneficial if we had tests for the item-level HATEOAS, especially considering it's a required feature for some front-end features to work.

I've fixed the web signal broken tests (unrelated) and I'm working on fixing the rest and implementing new tests cases for the new logic. I'll have it ready tomorrow.

MarkLark86
MarkLark86 reviewed Feb 5, 2025
View reviewed changes
superdesk/core/resources/resource_rest_endpoints.py
@@ -594,47 +594,7 @@ def _populate_item_hateoas(
}

# extract related links from the `resource_config.data_class` relations (annotations)
related_links = self._get_related_links_from_annotations(item)
if related_links:
if related_links := self.resource_config.data_class.get_related_links(item):
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Don't see := too often

Copy link
Author

Choose a reason for hiding this comment

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

I saw it once, long ago when it was proposed, then I forgot it was already there in python 3. I really like it, I think I will be using it more often 😃

superdesk/core/resources/model.py
@@ -148,6 +152,29 @@ def type(self) -> str:
#: Datetime the document was last updated
updated: Annotated[Optional[datetime], Field(alias="_updated")] = None

def __init_subclass__(cls) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nicely done 🤓

@eos87
Copy link
Author

eos87 commented Feb 7, 2025

Tests have been added. I wanted to add some more tests in the content_api to verify the functionality of the new Token Auth class but I don't want to spend more time in this ticket. Since the content_api will need much more work in terms of moving it to async, we can add those tests later on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarkLark86 MarkLark86 MarkLark86 approved these changes

Assignees
No one assigned
Labels
async
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@eos87 @MarkLark86