Skip to content

Commit 37e1166

Browse files
committed
feat: reason templates
1 parent 9da3ff1 commit 37e1166

File tree

8 files changed

+208
-5
lines changed

8 files changed

+208
-5
lines changed

api/access_config.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
import json
22
import logging
33
import os
4-
from typing import Any
4+
from typing import Any, Optional
55

66
logger = logging.getLogger(__name__)
77

88
# Define constants for AccessConfig JSON keys
99
BACKEND = "BACKEND"
1010
NAME_VALIDATION_PATTERN = "NAME_VALIDATION_PATTERN"
1111
NAME_VALIDATION_ERROR = "NAME_VALIDATION_ERROR"
12+
REASON_TEMPLATE = "REASON_TEMPLATE"
13+
REASON_TEMPLATE_REQUIRED = "REASON_TEMPLATE_REQUIRED"
1214

1315

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

2830

2931
class AccessConfig:
30-
def __init__(self, name_pattern: str, name_validation_error: str):
32+
def __init__(
33+
self,
34+
name_pattern: str,
35+
name_validation_error: str,
36+
reason_template: Optional[str] = None,
37+
reason_template_required: Optional[list[str]] = None,
38+
):
3139
self.name_pattern = name_pattern
3240
self.name_validation_error = name_validation_error
41+
self.reason_template = reason_template
42+
self.reason_template_required = reason_template_required
3343

3444

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

90+
# Reason template and required fields are optional
91+
reason_template = config.get(REASON_TEMPLATE)
92+
reason_template_required = config.get(REASON_TEMPLATE_REQUIRED)
93+
8094
return AccessConfig(
8195
name_pattern=name_pattern,
8296
name_validation_error=name_validation_error,
97+
reason_template=reason_template,
98+
reason_template_required=reason_template_required,
8399
)
84100

85101

api/operations/constraints/check_for_reason.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
selectinload,
66
)
77

8+
from api.access_config import get_access_config
89
from api.extensions import db
910
from api.models import AppGroup, OktaGroup, OktaGroupTagMap, RoleGroup, RoleGroupMap, Tag
1011
from api.models.tag import coalesce_constraints
@@ -44,7 +45,21 @@ def __init__(
4445

4546
@staticmethod
4647
def invalid_reason(reason: Optional[str]) -> bool:
47-
return reason is None or reason.strip() == ""
48+
if reason is None or reason.strip() == "":
49+
return True
50+
51+
# Check if the reason is just the template without modification
52+
access_config = get_access_config()
53+
if access_config.reason_template and reason.strip() == access_config.reason_template.strip():
54+
return True
55+
56+
# Check if the reason is missing required fields from the template
57+
if access_config.reason_template_required and reason:
58+
for required_field in access_config.reason_template_required:
59+
if required_field not in reason:
60+
return True
61+
62+
return False
4863

4964
def execute_for_group(self) -> Tuple[bool, str]:
5065
if self.invalid_reason(self.reason):

api/views/schemas/access_requests.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,38 @@
11
from datetime import datetime, timezone
22
from typing import Any, Dict, Optional
33

4-
from marshmallow import Schema, ValidationError, fields, post_load, validate
4+
from marshmallow import Schema, ValidationError, fields, post_load, validate, validates
5+
6+
from api.access_config import get_access_config
57

68

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

14+
@validates("reason")
15+
def validate_reason(self, reason: str) -> None:
16+
access_config = get_access_config()
17+
18+
# Check if reason is empty or only whitespace
19+
if not reason or not reason.strip():
20+
raise ValidationError("Reason is required")
21+
22+
# Check if reason is the same as the template
23+
if access_config.reason_template and reason.strip() == access_config.reason_template.strip():
24+
raise ValidationError(
25+
"Please fill out the template with your specific information instead of submitting the template as-is."
26+
)
27+
28+
# Check if required template fields are present
29+
if access_config.reason_template_required and reason:
30+
for required_field in access_config.reason_template_required:
31+
if required_field not in reason:
32+
raise ValidationError(
33+
f"The following required field is missing from your reason: {required_field}. Please fill it out before submitting your access request."
34+
)
35+
1236
@staticmethod
1337
def must_be_in_the_future(data: Optional[datetime]) -> None:
1438
if data and data < datetime.now():

src/config/accessConfig.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ export interface AccessConfig {
33
DEFAULT_ACCESS_TIME: string;
44
NAME_VALIDATION_PATTERN: string;
55
NAME_VALIDATION_ERROR: string;
6+
REASON_TEMPLATE: string;
7+
REASON_TEMPLATE_REQUIRED: string[];
68
}
79

810
// use the globally-injected ACCESS_CONFIG from src/globals.d.ts, typed to AccessConfig interface

src/config/loadAccessConfig.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ const DEFAULT_ACCESS_TIME = 'DEFAULT_ACCESS_TIME';
1111
const FRONTEND = 'FRONTEND';
1212
const NAME_VALIDATION_PATTERN = 'NAME_VALIDATION_PATTERN';
1313
const NAME_VALIDATION_ERROR = 'NAME_VALIDATION_ERROR';
14+
const REASON_TEMPLATE = 'REASON_TEMPLATE';
15+
const REASON_TEMPLATE_REQUIRED = 'REASON_TEMPLATE_REQUIRED';
1416

1517
class UndefinedConfigError extends Error {
1618
constructor(key, obj) {
@@ -59,6 +61,24 @@ function validate_override_config(overrideConfig) {
5961
`If ${NAME_VALIDATION_PATTERN} is present, ${NAME_VALIDATION_ERROR} must also be overridden.`,
6062
);
6163
}
64+
65+
if (REASON_TEMPLATE in overrideConfig && !(REASON_TEMPLATE_REQUIRED in overrideConfig)) {
66+
throw new AccessConfigValidationError(
67+
`If ${REASON_TEMPLATE} is present, ${REASON_TEMPLATE_REQUIRED} must also be overridden.`,
68+
);
69+
}
70+
// ensure that REASON_TEMPLATE has all required fields in REASON_TEMPLATE_REQUIRED
71+
if (REASON_TEMPLATE in overrideConfig && REASON_TEMPLATE_REQUIRED in overrideConfig) {
72+
const requiredFields = getConfig(overrideConfig, REASON_TEMPLATE_REQUIRED);
73+
const reasonTemplate = getConfig(overrideConfig, REASON_TEMPLATE);
74+
for (const field of requiredFields) {
75+
if (!(field in reasonTemplate)) {
76+
throw new AccessConfigValidationError(
77+
`Field '${field}' is required in ${REASON_TEMPLATE} when ${REASON_TEMPLATE_REQUIRED} is set.`,
78+
);
79+
}
80+
}
81+
}
6282
}
6383

6484
function loadOverrideConfig(accessConfig) {

src/pages/requests/Create.tsx

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@ function CreateRequestContainer(props: CreateRequestContainerProps) {
309309
group: props.group,
310310
until: accessConfig.DEFAULT_ACCESS_TIME,
311311
ownerOrMember: props.owner != null ? (props.owner ? 'owner' : 'member') : undefined,
312+
reason: accessConfig.REASON_TEMPLATE || '',
312313
}}
313314
onSuccess={(formData) => submit(formData)}>
314315
<DialogTitle>
@@ -434,7 +435,35 @@ function CreateRequestContainer(props: CreateRequestContainerProps) {
434435
name="reason"
435436
multiline
436437
rows={4}
437-
validation={{maxLength: 1024}}
438+
placeholder={accessConfig.REASON_TEMPLATE}
439+
validation={{
440+
required: 'Reason is required',
441+
maxLength: 1024,
442+
validate: (value: string) => {
443+
// Check if reason is empty or only whitespace
444+
if (!value || value.trim().length === 0) {
445+
return 'Reason is required';
446+
}
447+
448+
// Check if reason is the same as the template
449+
if (accessConfig.REASON_TEMPLATE && value.trim() === accessConfig.REASON_TEMPLATE.trim()) {
450+
return 'Please fill out the template with your specific information instead of submitting the template as-is.';
451+
}
452+
453+
// Check if required template fields are present
454+
if (accessConfig.REASON_TEMPLATE_REQUIRED && value) {
455+
const missingFields = accessConfig.REASON_TEMPLATE_REQUIRED.filter(
456+
(field: string) => !value.includes(field),
457+
);
458+
459+
if (missingFields.length > 0) {
460+
return `The following required fields are missing from your reason: ${missingFields.join(', ')}`;
461+
}
462+
}
463+
464+
return true;
465+
},
466+
}}
438467
parseError={(error) => {
439468
if (error?.message != '') {
440469
return error?.message ?? '';

src/pages/role_requests/Create.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as React from 'react';
22
import dayjs, {Dayjs} from 'dayjs';
33
import IsSameOrBefore from 'dayjs/plugin/isSameOrBefore';
44
import {useNavigate} from 'react-router-dom';
5+
import accessConfig from '../../config/accessConfig';
56
import Button from '@mui/material/Button';
67
import Dialog from '@mui/material/Dialog';
78
import DialogActions from '@mui/material/DialogActions';
@@ -462,6 +463,7 @@ function CreateRequestContainer(props: CreateRequestContainerProps) {
462463
name="reason"
463464
multiline
464465
rows={4}
466+
placeholder={accessConfig.REASON_TEMPLATE}
465467
validation={{maxLength: 1024}}
466468
parseError={(error) => {
467469
if (error?.message != '') {

tests/test_reason_template.py

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
from typing import Any
2+
3+
from flask import url_for
4+
from flask.testing import FlaskClient
5+
from flask_sqlalchemy import SQLAlchemy
6+
from pytest_mock import MockerFixture
7+
8+
from api.models import (
9+
OktaGroup,
10+
OktaUser,
11+
)
12+
from api.operations.constraints.check_for_reason import CheckForReason
13+
14+
15+
def test_invalid_reason_with_template(mocker: MockerFixture) -> None:
16+
"""Test that the template itself and templates with placeholders are considered invalid reasons."""
17+
18+
# Mock the access_config to return a specific template
19+
mock_access_config = mocker.patch("api.operations.constraints.check_for_reason.get_access_config")
20+
mock_config = mocker.MagicMock()
21+
mock_config.reason_template = (
22+
"Project: [Project Name]\nTicket: [Ticket ID]\nJustification: [Why is this access needed?]"
23+
)
24+
mock_config.reason_template_required = ["Project", "Ticket", "Justification"]
25+
mock_access_config.return_value = mock_config
26+
27+
# Test cases
28+
# 1. Empty reason
29+
assert CheckForReason.invalid_reason(None) is True
30+
assert CheckForReason.invalid_reason("") is True
31+
assert CheckForReason.invalid_reason(" ") is True
32+
33+
# 2. Template as-is (unchanged)
34+
template = "Project: [Project Name]\nTicket: [Ticket ID]\nJustification: [Why is this access needed?]"
35+
assert CheckForReason.invalid_reason(template) is True
36+
37+
# 3. Template with missing fields
38+
template = "Project: [Project Name]\nTicket: [Ticket ID]"
39+
assert CheckForReason.invalid_reason(template) is True
40+
41+
# 4. Template with all placeholders filled should be valid
42+
filled_template = "Project: My Project\nTicket: TICKET-123\nJustification: I need access to deploy code"
43+
assert CheckForReason.invalid_reason(filled_template) is False
44+
45+
# 5. Completely different invalid reason
46+
valid_reason = "I need access for the new project launch"
47+
assert CheckForReason.invalid_reason(valid_reason) is True
48+
49+
50+
def test_reason_validation_in_request_endpoint(
51+
client: FlaskClient,
52+
db: SQLAlchemy,
53+
mocker: MockerFixture,
54+
okta_group: OktaGroup,
55+
user: OktaUser,
56+
) -> None:
57+
"""Test that the API endpoints reject reasons that just contain the template."""
58+
59+
# Mock the access_config to return a specific template
60+
mock_access_config = mocker.patch("api.views.schemas.access_requests.get_access_config")
61+
mock_config = mocker.MagicMock()
62+
mock_config.reason_template = (
63+
"Project: [Project Name]\nTicket: [Ticket ID]\nJustification: [Why is this access needed?]"
64+
)
65+
mock_config.reason_template_required = ["Project", "Ticket", "Justification"]
66+
mock_access_config.return_value = mock_config
67+
68+
# Set up the group and user
69+
db.session.add(okta_group)
70+
db.session.add(user)
71+
db.session.commit()
72+
73+
# Try creating an access request with the template as reason
74+
template = "Project: [Project Name]\nTicket: [Ticket ID]"
75+
76+
data: dict[str, Any] = {
77+
"group_id": okta_group.id,
78+
"group_owner": False,
79+
"reason": template,
80+
}
81+
82+
# Create the access request
83+
access_request_url = url_for("api-access-requests.access_requests")
84+
rep = client.post(access_request_url, json=data)
85+
86+
# Should get rejected because the reason it is missing required information
87+
assert rep.status_code == 400
88+
89+
# Try again with a filled template
90+
filled_template = "Project: My Project\nTicket: TICKET-123\nJustification: I need access to deploy code"
91+
data["reason"] = filled_template
92+
93+
rep = client.post(access_request_url, json=data)
94+
# Should succeed with a properly filled template
95+
assert rep.status_code == 201

0 commit comments

Comments
 (0)