feat(flex-linux-setup): location of policy store file#2442
Conversation
Signed-off-by: Mustafa Baser <mbaser@mail.com>
devrimyatar
added
comp-flex-linux-setup
📝 WalkthroughWalkthroughInstaller now downloads a policy-store JSON from the configured external source instead of using the bundled template; it writes the file to Changes
Sequence Diagram(s)sequenceDiagram
participant User as Admin UI Setup
participant Installer as flex_setup.py
participant Repo as Remote Repo (GitHub)
participant FS as Filesystem (`policy_store_path`)
note over Installer,Repo: new optional download step
User->>Installer: start admin UI install
Installer->>Repo: download policy-store JSON (configured URL)
Repo-->>Installer: JSON content (or error)
Installer-->>FS: write to `policy_store_path` (if downloaded)
Installer->>FS: check `policy_store_path`
alt file exists
Installer->>Installer: attempt JSON parse
alt parse succeeds
Installer-->>User: render/import policy store
else parse fails
Installer-->>User: warn and skip render
end
else file missing
Installer-->>User: skip rendering policy store
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (5)📓 Common learnings📚 Learning: 2025-11-11T15:45:04.151ZApplied to files:
📚 Learning: 2025-11-11T15:17:34.651ZApplied to files:
📚 Learning: 2025-11-07T12:17:39.857ZApplied to files:
📚 Learning: 2025-11-07T12:55:26.241ZApplied to files:
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
flex-linux-setup/flex_linux_setup/flex_setup.py(1 hunks)flex-linux-setup/flex_linux_setup/templates/policy-store.json(0 hunks)
💤 Files with no reviewable changes (1)
- flex-linux-setup/flex_linux_setup/templates/policy-store.json
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
| ('https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/refs/heads/main/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json', | ||
| self.policy_store_path | ||
| ), |
There was a problem hiding this comment.
Add integrity verification for the security-critical policy store file.
The policy store file controls authorization policies for Admin UI and is downloaded from an external source without any checksum or signature verification. This poses a security risk if the source is compromised or subject to man-in-the-middle attacks.
Consider implementing one of the following:
- Verify a checksum after download (preferred for immutable releases)
- Verify a signature if the repository provides signed artifacts
- Pin to a specific commit hash in the URL to ensure the exact content is fetched
Example implementation for checksum verification:
# After line 382, add verification for policy store
expected_sha256 = "your_expected_checksum_here" # Define this
actual_sha256 = base.get_file_hash(self.policy_store_path, 'sha256')
if actual_sha256 != expected_sha256:
raise Exception(f"Policy store integrity check failed. Expected {expected_sha256}, got {actual_sha256}")Alternatively, modify the URL to pin to a specific commit:
-('https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/refs/heads/main/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json',
+('https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/<COMMIT_HASH>/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json',Based on learnings (audit context should include policy store URL).
🤖 Prompt for AI Agents
In flex-linux-setup/flex_linux_setup/flex_setup.py around lines 372-374, the
policy store JSON is downloaded without integrity checks; after the download
(around line ~382) add a SHA256 verification step that computes the file hash
(using base.get_file_hash or equivalent), compare it to a trusted expected
checksum stored as a constant or config value, and raise an exception/abort the
install if the hashes differ; alternatively, if signed artifacts are available
verify the signature instead or ensure the URL is pinned to an immutable commit
and document the expected checksum source.
🧩 Analysis chain
Verify the policy store URL is accessible and consider using an immutable reference.
The policy store is downloaded from the main branch, which is mutable and can lead to non-reproducible builds. For stability and security, consider using a specific git tag or commit hash in the URL path instead of refs/heads/main.
Additionally, verify that the URL is accessible and that the filename hash 2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5 is correct (it appears to be 44 characters, which is an unusual hash length).
Run the following script to verify URL accessibility:
🏁 Script executed:
#!/bin/bash
# Verify the policy store URL is accessible
policy_store_url="https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/refs/heads/main/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json"
echo "Testing URL accessibility..."
http_code=$(curl -s -o /dev/null -w "%{http_code}" "$policy_store_url")
if [ "$http_code" = "200" ]; then
echo "✓ URL is accessible (HTTP $http_code)"
echo ""
echo "File size and content type:"
curl -sI "$policy_store_url" | grep -E "(Content-Length|Content-Type)"
echo ""
echo "First 200 characters of content:"
curl -s "$policy_store_url" | head -c 200
else
echo "✗ URL returned HTTP $http_code"
fi
echo -e "\n\nChecking repository structure..."
curl -s "https://api.github.com/repos/GluuFederation/GluuFlexAdminUIPolicyStore/contents/" | jq -r '.[].name' 2>/dev/null || echo "Could not fetch repository contents"Length of output: 438
🏁 Script executed:
#!/bin/bash
echo "=== 1. Finding base.download() implementation ==="
rg -n "def download|\.download\(" -A 10 -B 2 flex-linux-setup/flex_linux_setup/ | head -100
echo ""
echo "=== 2. Finding all URLs in source_files ==="
rg -n "https://" flex-linux-setup/flex_linux_setup/flex_setup.py | grep -E "(source_files|self\.source_files|http)" -A 1 -B 1
echo ""
echo "=== 3. Checking for checksum/signature validation ==="
rg -n "checksum|hash|verify|signature|integrity" flex-linux-setup/flex_linux_setup/flex_setup.py
echo ""
echo "=== 4. Examining download loop and error handling around policy_store ==="
rg -n "for.*source_files|policy_store" flex-linux-setup/flex_linux_setup/flex_setup.py -B 2 -A 5Length of output: 8404
🏁 Script executed:
#!/bin/bash
echo "=== 1. Finding base module import ==="
rg -n "^import|^from" flex-linux-setup/flex_linux_setup/flex_setup.py | head -30
echo ""
echo "=== 2. Searching for base.download definition ==="
fd . flex-linux-setup -type f -name "*.py" -exec grep -l "def download" {} \;
echo ""
echo "=== 3. Checking base module for error handling ==="
fd . flex-linux-setup -type f -name "base.py" -exec cat {} \;Length of output: 1464
🏁 Script executed:
#!/bin/bash
echo "=== 1. Finding base.py module ==="
find flex-linux-setup -name "base.py" -type f
echo ""
echo "=== 2. Extracting base.download() function ==="
find flex-linux-setup -name "base.py" -type f -exec grep -A 20 "def download" {} +
echo ""
echo "=== 3. Checking for exception handling in download flow ==="
find flex-linux-setup -name "base.py" -type f -exec cat {} \;Length of output: 142
🏁 Script executed:
#!/bin/bash
echo "=== 1. Check if policy_store_path file is validated before use ==="
rg -n "policy_store_path|renderTemplateInOut" flex-linux-setup/flex_linux_setup/flex_setup.py -B 3 -A 3
echo ""
echo "=== 2. Look for any configuration or version control strategy doc ==="
find flex-linux-setup -name "README*" -o -name "CHANGELOG*" -o -name "*.md" | head -10
echo ""
echo "=== 3. Check error handling patterns in download_files method ==="
sed -n '344,386p' flex-linux-setup/flex_linux_setup/flex_setup.py
echo ""
echo "=== 4. See if there's version or branch configuration ==="
rg -n "FLEX_BRANCH|app_versions|refs/heads" flex-linux-setup/flex_linux_setup/flex_setup.py | head -20Length of output: 6205
Address missing error handling and inconsistent branch reference for policy store.
The policy store URL is hardcoded to the mutable main branch, unlike other downloads which use configurable branches (JANS_BRANCH, FLEX_BRANCH). More critically, if the download fails, line 576 will execute renderTemplateInOut() on a non-existent file without validation, causing the setup to crash.
Recommendations:
- Add error handling around the download at lines 380–382 or validate file existence before line 576
- Either make the policy store URL configurable via CLI (like other branches) or pin it to a specific release tag/commit hash for reproducibility and stability
- Consider adding checksum validation for this security-critical file before use
🤖 Prompt for AI Agents
In flex-linux-setup/flex_linux_setup/flex_setup.py around lines 372-374 (and
referencing the download area ~380-382 and later renderTemplateInOut usage at
line 576), the policy store URL is hardcoded to the mutable "main" branch and
there is no error handling or validation before renderTemplateInOut is called.
Change the URL to use an existing branch/variable (e.g., FLEX_BRANCH or a new
CLI option) or pin it to a release tag/commit for reproducibility; wrap the
download call with try/except (or check the HTTP status) and raise or log a
clear error if it fails; before calling renderTemplateInOut at line 576,
validate that self.policy_store_path exists and is a readable file (and
optionally verify a checksum) and abort with a helpful error if validation
fails.
There was a problem hiding this comment.
@coderabbitai This file https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/refs/heads/main/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json is updated occasionally and not static, therefor we don't (and can't) have a checksum hash for that file. Additionally, we should download from main branch since all commits are meged into main.
There was a problem hiding this comment.
@devrimyatar, thank you for the clarification! I understand that the policy store needs to be fetched from main since it's regularly updated, and static checksums aren't feasible for a living configuration file.
I notice the code already has a good safeguard at line 576 with if os.path.exists(self.policy_store_path): to handle cases where the file doesn't exist, which addresses the concern about rendering a non-existent file.
Given the dynamic nature of this file, here are a couple of optional suggestions to consider for robustness:
- Add explicit error handling for the download to provide clear feedback if it fails:
try:
base.download(policy_store_url, self.policy_store_path, verbose=True)
except Exception as e:
print(f"Warning: Failed to download policy store: {e}")
print("Admin UI will continue without cedarling policy store integration")- Basic JSON validation after download to catch corrupted downloads early:
if os.path.exists(self.policy_store_path):
try:
with open(self.policy_store_path) as f:
json.load(f) # Validates JSON format
config_api_installer.renderTemplateInOut(self.policy_store_path, self.templates_dir, admin_ui_config_dir)
except json.JSONDecodeError as e:
print(f"Warning: Downloaded policy store is not valid JSON: {e}")These are optional enhancements for better visibility when issues occur, but the current implementation with the existence check is already reasonably safe.
🧠 Learnings used
Learnt from: devrimyatar
Repo: GluuFederation/flex PR: 2440
File: flex-linux-setup/flex_linux_setup/flex_setup.py:519-519
Timestamp: 2025-11-11T15:17:34.639Z
Learning: In flex-linux-setup/flex_linux_setup/flex_setup.py, the webhook LDIF (aui_webhook.ldif) should only be imported during fresh Admin UI installations, not during updates. The import is correctly guarded by the check `client_check_result['2001.'] == -1` which ensures it only runs when the Admin UI Web Client doesn't already exist. Docker update workflows differ from Linux update workflows in this regard.
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
There was a problem hiding this comment.
- You suggest error handling for fle download. But is is done in function
base.download()(see https://github.com/JanssenProject/jans/blob/main/jans-linux-setup/jans_setup/setup_app/utils/base.py#L342-L374) - I checked validty of JSON in commit
b3e68a2
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch the commit diff to see the JSON validation implementation
gh api repos/GluuFederation/flex/commits/b3e68a27663df8491cc615dfaed2aa17d55c985a --jq '.files[] | select(.filename | contains("flex_setup.py")) | .patch'
Length of output: 955
@devrimyatar, thank you for the clarification and for implementing the JSON validation!
You're absolutely right on both points:
-
Error handling: I see that
base.download()already includes comprehensive error handling with retries and proper exception management. I wasn't aware of that implementation detail - thank you for the reference. -
JSON validation: Perfect! I can see from the commit that you've added exactly the validation I suggested:
- JSON format validation with
json.load() - Try/except block to catch
JSONDecodeError - Warning message if validation fails
- Graceful continuation without crashing the setup
- JSON format validation with
The implementation now has multiple layers of protection:
- ✅ Download error handling in
base.download() - ✅ File existence check with
os.path.exists() - ✅ JSON integrity validation before use
This looks solid and handles the dynamic nature of the policy store file appropriately. The setup will gracefully continue even if the policy store is temporarily unavailable or corrupted, which is the right behavior.
✏️ Learnings added
Learnt from: devrimyatar
Repo: GluuFederation/flex PR: 2442
File: flex-linux-setup/flex_linux_setup/flex_setup.py:372-374
Timestamp: 2025-11-11T15:45:04.127Z
Learning: In flex-linux-setup/flex_linux_setup/flex_setup.py, the Admin UI policy store JSON file is downloaded from the main branch of the GluuFlexAdminUIPolicyStore repository (https://raw.githubusercontent.com/GluuFederation/GluuFlexAdminUIPolicyStore/refs/heads/main/2fb50e468d9dfefa142d1fce4fa9747efbd3a0f08de5.json). This file is regularly updated, so static checksums are not feasible. The download is handled by base.download() which includes built-in error handling, and JSON validation is performed after download to ensure file integrity before rendering the template.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: devrimyatar
Repo: GluuFederation/flex PR: 2440
File: flex-linux-setup/flex_linux_setup/flex_setup.py:519-519
Timestamp: 2025-11-11T15:17:34.639Z
Learning: In flex-linux-setup/flex_linux_setup/flex_setup.py, the webhook LDIF (aui_webhook.ldif) should only be imported during fresh Admin UI installations, not during updates. The import is correctly guarded by the check `client_check_result['2001.'] == -1` which ensures it only runs when the Admin UI Web Client doesn't already exist. Docker update workflows differ from Linux update workflows in this regard.
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
… codrabbit Signed-off-by: Mustafa Baser <mbaser@mail.com>
…uggested by codrabbit Signed-off-by: Mustafa Baser <mbaser@mail.com>
Signed-off-by: Mustafa Baser <mbaser@mail.com>
Signed-off-by: Mustafa Baser <mbaser@mail.com>
|
2 participants
Closes #2441
Summary by CodeRabbit
Chores
Bug Fixes