Skip to content
Closed
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
20 changes: 18 additions & 2 deletions api/access_config.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import json
import logging
import os
from typing import Any
from typing import Any, Optional

logger = logging.getLogger(__name__)

# Define constants for AccessConfig JSON keys
BACKEND = "BACKEND"
NAME_VALIDATION_PATTERN = "NAME_VALIDATION_PATTERN"
NAME_VALIDATION_ERROR = "NAME_VALIDATION_ERROR"
REASON_TEMPLATE = "REASON_TEMPLATE"
REASON_TEMPLATE_REQUIRED = "REASON_TEMPLATE_REQUIRED"


class UndefinedConfigKeyError(Exception):
Expand All @@ -27,9 +29,17 @@ def __init__(self, error: str):


class AccessConfig:
def __init__(self, name_pattern: str, name_validation_error: str):
def __init__(
self,
name_pattern: str,
name_validation_error: str,
reason_template: Optional[str] = None,
reason_template_required: Optional[list[str]] = None,
):
self.name_pattern = name_pattern
self.name_validation_error = name_validation_error
self.reason_template = reason_template
self.reason_template_required = reason_template_required


def _get_config_value(config: dict[str, Any], key: str) -> Any:
Expand Down Expand Up @@ -77,9 +87,15 @@ def _load_access_config() -> AccessConfig:
name_pattern = _get_config_value(config, NAME_VALIDATION_PATTERN)
name_validation_error = _get_config_value(config, NAME_VALIDATION_ERROR)

# Reason template and required fields are optional
reason_template = config.get(REASON_TEMPLATE)
reason_template_required = config.get(REASON_TEMPLATE_REQUIRED)

return AccessConfig(
name_pattern=name_pattern,
name_validation_error=name_validation_error,
reason_template=reason_template,
reason_template_required=reason_template_required,
)


Expand Down
17 changes: 16 additions & 1 deletion api/operations/constraints/check_for_reason.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
selectinload,
)

from api.access_config import get_access_config
from api.extensions import db
from api.models import AppGroup, OktaGroup, OktaGroupTagMap, RoleGroup, RoleGroupMap, Tag
from api.models.tag import coalesce_constraints
Expand Down Expand Up @@ -44,7 +45,21 @@ def __init__(

@staticmethod
def invalid_reason(reason: Optional[str]) -> bool:
return reason is None or reason.strip() == ""
if reason is None or reason.strip() == "":
return True

# Check if the reason is just the template without modification
access_config = get_access_config()
if access_config.reason_template and reason.strip() == access_config.reason_template.strip():
return True

# Check if the reason is missing required fields from the template
if access_config.reason_template_required and reason:
for required_field in access_config.reason_template_required:
if required_field not in reason:
return True

return False

def execute_for_group(self) -> Tuple[bool, str]:
if self.invalid_reason(self.reason):
Expand Down
26 changes: 25 additions & 1 deletion api/views/schemas/access_requests.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,38 @@
from datetime import datetime, timezone
from typing import Any, Dict, Optional

from marshmallow import Schema, ValidationError, fields, post_load, validate
from marshmallow import Schema, ValidationError, fields, post_load, validate, validates

from api.access_config import get_access_config


class CreateAccessRequestSchema(Schema):
group_id = fields.String(validate=validate.Length(equal=20), required=True, load_only=True)
group_owner = fields.Boolean(load_default=False, load_only=True)
reason = fields.String(validate=validate.Length(max=1024), load_only=True)

@validates("reason")
def validate_reason(self, reason: str) -> None:
access_config = get_access_config()

# Check if reason is empty or only whitespace
if not reason or not reason.strip():
raise ValidationError("Reason is required")

# Check if reason is the same as the template
if access_config.reason_template and reason.strip() == access_config.reason_template.strip():
raise ValidationError(
"Please fill out the template with your specific information instead of submitting the template as-is."
)

# Check if required template fields are present
if access_config.reason_template_required and reason:
for required_field in access_config.reason_template_required:
if required_field not in reason:
raise ValidationError(
f"The following required field is missing from your reason: {required_field}. Please fill it out before submitting your access request."
)

@staticmethod
def must_be_in_the_future(data: Optional[datetime]) -> None:
if data and data < datetime.now():
Expand Down
2 changes: 2 additions & 0 deletions src/config/accessConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ export interface AccessConfig {
DEFAULT_ACCESS_TIME: string;
NAME_VALIDATION_PATTERN: string;
NAME_VALIDATION_ERROR: string;
REASON_TEMPLATE: string;
REASON_TEMPLATE_REQUIRED: string[];
}

// use the globally-injected ACCESS_CONFIG from src/globals.d.ts, typed to AccessConfig interface
Expand Down
20 changes: 20 additions & 0 deletions src/config/loadAccessConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ const DEFAULT_ACCESS_TIME = 'DEFAULT_ACCESS_TIME';
const FRONTEND = 'FRONTEND';
const NAME_VALIDATION_PATTERN = 'NAME_VALIDATION_PATTERN';
const NAME_VALIDATION_ERROR = 'NAME_VALIDATION_ERROR';
const REASON_TEMPLATE = 'REASON_TEMPLATE';
const REASON_TEMPLATE_REQUIRED = 'REASON_TEMPLATE_REQUIRED';

class UndefinedConfigError extends Error {
constructor(key, obj) {
Expand Down Expand Up @@ -59,6 +61,24 @@ function validate_override_config(overrideConfig) {
`If ${NAME_VALIDATION_PATTERN} is present, ${NAME_VALIDATION_ERROR} must also be overridden.`,
);
}

if (REASON_TEMPLATE in overrideConfig && !(REASON_TEMPLATE_REQUIRED in overrideConfig)) {
throw new AccessConfigValidationError(
`If ${REASON_TEMPLATE} is present, ${REASON_TEMPLATE_REQUIRED} must also be overridden.`,
);
}
// ensure that REASON_TEMPLATE has all required fields in REASON_TEMPLATE_REQUIRED
if (REASON_TEMPLATE in overrideConfig && REASON_TEMPLATE_REQUIRED in overrideConfig) {
const requiredFields = getConfig(overrideConfig, REASON_TEMPLATE_REQUIRED);
const reasonTemplate = getConfig(overrideConfig, REASON_TEMPLATE);
for (const field of requiredFields) {
if (!(field in reasonTemplate)) {
throw new AccessConfigValidationError(
`Field '${field}' is required in ${REASON_TEMPLATE} when ${REASON_TEMPLATE_REQUIRED} is set.`,
);
}
}
}
}

function loadOverrideConfig(accessConfig) {
Expand Down
31 changes: 30 additions & 1 deletion src/pages/requests/Create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ function CreateRequestContainer(props: CreateRequestContainerProps) {
group: props.group,
until: accessConfig.DEFAULT_ACCESS_TIME,
ownerOrMember: props.owner != null ? (props.owner ? 'owner' : 'member') : undefined,
reason: accessConfig.REASON_TEMPLATE || '',
}}
onSuccess={(formData) => submit(formData)}>
<DialogTitle>
Expand Down Expand Up @@ -434,7 +435,35 @@ function CreateRequestContainer(props: CreateRequestContainerProps) {
name="reason"
multiline
rows={4}
validation={{maxLength: 1024}}
placeholder={accessConfig.REASON_TEMPLATE}
validation={{
required: 'Reason is required',
maxLength: 1024,
validate: (value: string) => {
// Check if reason is empty or only whitespace
if (!value || value.trim().length === 0) {
return 'Reason is required';
}

// Check if reason is the same as the template
if (accessConfig.REASON_TEMPLATE && value.trim() === accessConfig.REASON_TEMPLATE.trim()) {
return 'Please fill out the template with your specific information instead of submitting the template as-is.';
}

// Check if required template fields are present
if (accessConfig.REASON_TEMPLATE_REQUIRED && value) {
const missingFields = accessConfig.REASON_TEMPLATE_REQUIRED.filter(
(field: string) => !value.includes(field),
);

if (missingFields.length > 0) {
return `The following required fields are missing from your reason: ${missingFields.join(', ')}`;
}
}

return true;
},
}}
parseError={(error) => {
if (error?.message != '') {
return error?.message ?? '';
Expand Down
2 changes: 2 additions & 0 deletions src/pages/role_requests/Create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import dayjs, {Dayjs} from 'dayjs';
import IsSameOrBefore from 'dayjs/plugin/isSameOrBefore';
import {useNavigate} from 'react-router-dom';
import accessConfig from '../../config/accessConfig';
import Button from '@mui/material/Button';
import Dialog from '@mui/material/Dialog';
import DialogActions from '@mui/material/DialogActions';
Expand Down Expand Up @@ -462,6 +463,7 @@ function CreateRequestContainer(props: CreateRequestContainerProps) {
name="reason"
multiline
rows={4}
placeholder={accessConfig.REASON_TEMPLATE}
validation={{maxLength: 1024}}
parseError={(error) => {
if (error?.message != '') {
Expand Down
95 changes: 95 additions & 0 deletions tests/test_reason_template.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
from typing import Any

from flask import url_for
from flask.testing import FlaskClient
from flask_sqlalchemy import SQLAlchemy
from pytest_mock import MockerFixture

from api.models import (
OktaGroup,
OktaUser,
)
from api.operations.constraints.check_for_reason import CheckForReason


def test_invalid_reason_with_template(mocker: MockerFixture) -> None:
"""Test that the template itself and templates with placeholders are considered invalid reasons."""

# Mock the access_config to return a specific template
mock_access_config = mocker.patch("api.operations.constraints.check_for_reason.get_access_config")
mock_config = mocker.MagicMock()
mock_config.reason_template = (
"Project: [Project Name]\nTicket: [Ticket ID]\nJustification: [Why is this access needed?]"
)
mock_config.reason_template_required = ["Project", "Ticket", "Justification"]
mock_access_config.return_value = mock_config

# Test cases
# 1. Empty reason
assert CheckForReason.invalid_reason(None) is True
assert CheckForReason.invalid_reason("") is True
assert CheckForReason.invalid_reason(" ") is True

# 2. Template as-is (unchanged)
template = "Project: [Project Name]\nTicket: [Ticket ID]\nJustification: [Why is this access needed?]"
assert CheckForReason.invalid_reason(template) is True

# 3. Template with missing fields
template = "Project: [Project Name]\nTicket: [Ticket ID]"
assert CheckForReason.invalid_reason(template) is True

# 4. Template with all placeholders filled should be valid
filled_template = "Project: My Project\nTicket: TICKET-123\nJustification: I need access to deploy code"
assert CheckForReason.invalid_reason(filled_template) is False

# 5. Completely different invalid reason
valid_reason = "I need access for the new project launch"
assert CheckForReason.invalid_reason(valid_reason) is True


def test_reason_validation_in_request_endpoint(
client: FlaskClient,
db: SQLAlchemy,
mocker: MockerFixture,
okta_group: OktaGroup,
user: OktaUser,
) -> None:
"""Test that the API endpoints reject reasons that just contain the template."""

# Mock the access_config to return a specific template
mock_access_config = mocker.patch("api.views.schemas.access_requests.get_access_config")
mock_config = mocker.MagicMock()
mock_config.reason_template = (
"Project: [Project Name]\nTicket: [Ticket ID]\nJustification: [Why is this access needed?]"
)
mock_config.reason_template_required = ["Project", "Ticket", "Justification"]
mock_access_config.return_value = mock_config

# Set up the group and user
db.session.add(okta_group)
db.session.add(user)
db.session.commit()

# Try creating an access request with the template as reason
template = "Project: [Project Name]\nTicket: [Ticket ID]"

data: dict[str, Any] = {
"group_id": okta_group.id,
"group_owner": False,
"reason": template,
}

# Create the access request
access_request_url = url_for("api-access-requests.access_requests")
rep = client.post(access_request_url, json=data)

# Should get rejected because the reason it is missing required information
assert rep.status_code == 400

# Try again with a filled template
filled_template = "Project: My Project\nTicket: TICKET-123\nJustification: I need access to deploy code"
data["reason"] = filled_template

rep = client.post(access_request_url, json=data)
# Should succeed with a properly filled template
assert rep.status_code == 201