Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
59 changes: 58 additions & 1 deletion tests/unit/manage/views/test_organizations.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@
OrganizationRoleType,
OrganizationType,
)
from warehouse.packaging import Project
from warehouse.packaging import IProjectService, Project
from warehouse.packaging.interfaces import TooManyProjectsCreated
from warehouse.utils.paginate import paginate_url_factory


Expand Down Expand Up @@ -2135,6 +2136,62 @@ def test_add_organization_project_new_project_name_conflict(
]
assert len(organization.projects) == 1

@pytest.mark.usefixtures("_enable_organizations")
@pytest.mark.parametrize(
("resets_in", "expected_error"),
[
(
datetime.timedelta(seconds=42),
"Too many new projects created. Try again in 42 seconds.",
),
(None, "Too many new projects created. Try again later."),
],
)
def test_add_organization_project_new_project_ratelimited(
self,
db_request,
pyramid_user,
organization_service,
monkeypatch,
resets_in,
expected_error,
):
organization = OrganizationFactory.create()
OrganizationRoleFactory.create(
organization=organization, user=db_request.user, role_name="Owner"
)

add_organization_project_obj = pretend.stub(
add_existing_project=pretend.stub(data=False),
new_project_name=pretend.stub(data="some-new-project", errors=[]),
validate=lambda *a, **kw: True,
)
monkeypatch.setattr(
org_views,
"AddOrganizationProjectForm",
lambda *a, **kw: add_organization_project_obj,
)

project_service = pretend.stub(
create_project=pretend.raiser(TooManyProjectsCreated(resets_in=resets_in)),
)
db_request.find_service = lambda svc, **kw: (
project_service if svc is IProjectService else organization_service
)

view = org_views.ManageOrganizationProjectsViews(organization, db_request)
result = view.add_organization_project()

assert result == {
"organization": organization,
"active_projects": view.active_projects,
"projects_owned": set(),
"projects_sole_owned": set(),
"add_organization_project_form": add_organization_project_obj,
}
assert add_organization_project_obj.new_project_name.errors == [expected_error]
assert len(organization.projects) == 0


class TestManageOrganizationRoles:
@pytest.mark.usefixtures("_enable_organizations")
Expand Down
53 changes: 52 additions & 1 deletion tests/unit/oidc/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import json

from datetime import datetime
from datetime import datetime, timedelta

import pretend
import pytest
Expand Down Expand Up @@ -30,6 +30,7 @@
)
from warehouse.organizations.models import OrganizationProject
from warehouse.packaging import services
from warehouse.packaging.interfaces import IProjectService, TooManyProjectsCreated
from warehouse.packaging.models import Project
from warehouse.rate_limiting.interfaces import IRateLimiter

Expand Down Expand Up @@ -456,6 +457,56 @@ def test_mint_token_pending_publisher_project_already_exists(db_request):
assert oidc_service.find_publisher.calls == [pretend.call(claims, pending=True)]


@pytest.mark.parametrize(
("resets_in", "expected_description"),
[
(
timedelta(seconds=42),
"Too many new projects created. Try again in 42 seconds.",
),
(None, "Too many new projects created. Try again later."),
],
)
def test_mint_token_pending_publisher_project_create_ratelimited(
db_request, resets_in, expected_description
):
pending_publisher = PendingGitHubPublisherFactory.create(
project_name="does-not-exist",
)

db_request.flags.disallow_oidc = lambda f=None: False

claims = {"iss": "https://none"}
oidc_service = pretend.stub(
verify_jwt_signature=pretend.call_recorder(
lambda token, issuer_url=None: claims
),
find_publisher=pretend.call_recorder(
lambda claims, pending=False: pending_publisher
),
)
project_service = pretend.stub(
create_project=pretend.raiser(TooManyProjectsCreated(resets_in=resets_in)),
)
db_request.find_service = lambda svc, **kw: (
project_service if svc is IProjectService else oidc_service
)

resp = views.mint_token(
oidc_service, DUMMY_GITHUB_OIDC_JWT, claims["iss"], db_request
)
assert db_request.response.status_code == 422
assert resp == {
"message": "Token request failed",
"errors": [
{
"code": "too-many-projects",
"description": expected_description,
}
],
}


def test_mint_token_from_oidc_pending_publisher_ok(monkeypatch, db_request):
user = UserFactory.create()

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/packaging/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -1030,7 +1030,7 @@ def test_check_project_test_new_disallowed(self, db_request):
service = ProjectService(session=db_request.db)

with pytest.raises(HTTPForbidden) as exc:
service.create_project("foo", pretend.stub(), db_request, ratelimited=False)
service.create_project("foo", pretend.stub(id=pretend.stub()), db_request)

resp = exc.value
assert resp.status_code == 403
Expand Down
30 changes: 15 additions & 15 deletions warehouse/locale/messages.pot

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 11 additions & 1 deletion warehouse/manage/views/organizations.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
TermsOfServiceEngagement,
)
from warehouse.packaging import IProjectService, Project, Role
from warehouse.packaging.interfaces import TooManyProjectsCreated
from warehouse.packaging.models import JournalEntry, ProjectFactory
from warehouse.subscriptions import IBillingService, ISubscriptionService
from warehouse.subscriptions.services import MockStripeBillingService
Expand Down Expand Up @@ -881,11 +882,20 @@ def add_organization_project(self):
self.request.user,
request=self.request,
creator_is_owner=False,
ratelimited=False,
)
Comment on lines 882 to 885
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Catch project rate-limit errors when adding org projects

This call now goes through ProjectService.create_project() with rate limiting enabled, but this view still only catches HTTPException. When a user has exceeded project.create.* limits, create_project() raises TooManyProjectsCreated (a RateLimiterException, not an HTTPException), so the exception escapes and turns a normal validation failure into a 500 for organization project creation attempts.

Useful? React with 👍 / 👎.

except HTTPException as exc:
form.new_project_name.errors.append(exc.detail)
return default_response
except TooManyProjectsCreated as exc:
retry = (
f"Try again in {int(exc.resets_in.total_seconds())} seconds."
if exc.resets_in is not None
else "Try again later."
)
form.new_project_name.errors.append(
f"Too many new projects created. {retry}"
)
return default_response

# Add project to organization.
self.organization_service.add_organization_project(
Expand Down
18 changes: 16 additions & 2 deletions warehouse/oidc/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
OIDC_ISSUER_SERVICE_NAMES,
lookup_custom_issuer_type,
)
from warehouse.packaging.interfaces import IProjectService
from warehouse.packaging.interfaces import IProjectService, TooManyProjectsCreated
from warehouse.packaging.models import ProjectFactory
from warehouse.rate_limiting.interfaces import IRateLimiter

Expand Down Expand Up @@ -225,14 +225,28 @@ def mint_token(
pending_publisher.added_by,
request,
creator_is_owner=pending_publisher.organization_id is None,
ratelimited=False,
organization_id=pending_publisher.organization_id,
)
Comment on lines 225 to 229
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Catch project rate-limit errors in pending publisher minting

Pending-publisher token minting now invokes rate-limited project creation, but this block only handles HTTPException. If the pending publisher owner has hit project creation limits, create_project() raises TooManyProjectsCreated (not an HTTPException), which is unhandled here and results in a server error instead of a structured API error response.

Useful? React with 👍 / 👎.

except HTTPException as exc:
return _invalid(
errors=[{"code": "invalid-payload", "description": str(exc)}],
request=request,
)
except TooManyProjectsCreated as exc:
retry = (
f"Try again in {int(exc.resets_in.total_seconds())} seconds."
if exc.resets_in is not None
else "Try again later."
)
return _invalid(
errors=[
{
"code": "too-many-projects",
"description": f"Too many new projects created. {retry}",
}
],
request=request,
)

# Reify the pending publisher against the newly created project
reified_publisher = oidc_service.reify_pending_publisher(
Expand Down
11 changes: 3 additions & 8 deletions warehouse/packaging/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,9 +437,7 @@ def _check_ratelimits(self, request, creator):
tags=["ratelimiter:user"],
)
raise TooManyProjectsCreated(
resets_in=self.ratelimiters["project.create.user"].resets_in(
request.remote_addr
)
resets_in=self.ratelimiters["project.create.user"].resets_in(creator.id)
)

def _hit_ratelimits(self, request, creator):
Expand Down Expand Up @@ -500,11 +498,9 @@ def create_project(
request,
*,
creator_is_owner=True,
ratelimited=True,
organization_id=None,
):
if ratelimited:
self._check_ratelimits(request, creator)
self._check_ratelimits(request, creator)

# Check for AdminFlag set by a PyPI Administrator disabling new project
# registration, reasons for this include Spammers, security
Expand Down Expand Up @@ -716,8 +712,7 @@ def create_project(
)
request.db.delete(stale_publisher)

if ratelimited:
self._hit_ratelimits(request, creator)
self._hit_ratelimits(request, creator)
return project


Expand Down
Loading