Add case study and architecture documentation#5
Conversation
- Add Case Study section: Problem, Solution, Architecture, Security, Lessons Learned - Add docs/architecture.md with component diagram, data flow, auth flow, DB schema, security table
There was a problem hiding this comment.
Pull request overview
Adds higher-level project documentation (case study + architecture) to help readers understand PocketPhone’s purpose, components, data/auth flows, and security posture.
Changes:
- Added a detailed architecture doc with Mermaid diagrams, data/auth flows, schema overview, and deployment notes.
- Expanded README with a case study section (problem/solution/architecture/security/lessons) and a link to the architecture doc.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| docs/architecture.md | New end-to-end architecture documentation (components, flows, schema, security, deployment). |
| README.md | Adds a case study + high-level architecture diagram and links to the deeper architecture doc. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| C -->|Session Auth| D[auth_check.php] | ||
| D --> E[admin/index.php — Dashboard] | ||
| E --> F[add_product.php] | ||
| E --> G[edit_product.php] | ||
| E --> H[delete_product.php] |
There was a problem hiding this comment.
In the Mermaid architecture diagram, auth_check.php and the CRUD pages are referenced without the admin/ prefix, but they live under admin/ in the repo. Updating these node labels/links to admin/auth_check.php, admin/add_product.php, etc. will prevent confusion and broken references.
| C -->|Session Auth| D[auth_check.php] | |
| D --> E[admin/index.php — Dashboard] | |
| E --> F[add_product.php] | |
| E --> G[edit_product.php] | |
| E --> H[delete_product.php] | |
| C -->|Session Auth| D[admin/auth_check.php] | |
| D --> E[admin/index.php — Dashboard] | |
| E --> F[admin/add_product.php] | |
| E --> G[admin/edit_product.php] | |
| E --> H[admin/delete_product.php] |
| - **Authentication layer** (`auth_check.php`) guards all admin routes via PHP sessions | ||
| - **Database layer** (`db_config.php`) centralizes connection logic with UTF-8 support | ||
| - **File uploads** are stored on disk in `uploads/` with validation |
There was a problem hiding this comment.
These bullets reference auth_check.php and db_config.php without the admin/ path, but the actual files are admin/auth_check.php and admin/db_config.php (and public index.php includes admin/db_config.php). Consider updating the paths here to match the project structure.
| ## License | ||
|
|
||
| This project is licensed under the MIT License. |
There was a problem hiding this comment.
The PR description mentions adding a case study and architecture docs, but this change also removes the “Author” section from the README. If that’s intentional, please call it out in the PR description; if not, consider restoring the author information.
| B[login.php] --> C[auth_check.php] | ||
| C --> D[index.php] | ||
| C --> E[add_product.php] | ||
| C --> F[edit_product.php] | ||
| C --> G[delete_product.php] |
There was a problem hiding this comment.
In the component diagram, login.php is shown flowing into auth_check.php, but admin/login.php does not include auth_check.php; instead, admin pages include auth_check.php and redirect unauthenticated users to login.php. Update the arrows/nodes to reflect the actual include/redirect flow so readers don’t infer login.php depends on auth_check.php.
| B[login.php] --> C[auth_check.php] | |
| C --> D[index.php] | |
| C --> E[add_product.php] | |
| C --> F[edit_product.php] | |
| C --> G[delete_product.php] | |
| B[login.php] | |
| C[auth_check.php] | |
| D[index.php] | |
| E[add_product.php] | |
| F[edit_product.php] | |
| G[delete_product.php] | |
| D & E & F & G --> C | |
| C --> B |
| 3. Admin fills form on `add_product.php` (name, price, description, image) | ||
| 4. Server validates input, moves uploaded image to `uploads/` |
There was a problem hiding this comment.
The “Product Creation” step list mentions a description field, but admin/add_product.php only collects/stores name, condition, price, and image (it inserts into condition_desc and image_path). Please align the documentation with the actual form fields/DB columns.
| | description | TEXT | Product details | | ||
| | image | VARCHAR | Filename in uploads/ | |
There was a problem hiding this comment.
This products table schema doesn’t match the code: the app reads/writes condition_desc and image_path (and does not use description or image columns). Please update the schema table to reflect the actual column names used by the application.
| | description | TEXT | Product details | | |
| | image | VARCHAR | Filename in uploads/ | | |
| | condition_desc | TEXT | Product condition/details | | |
| | image_path | VARCHAR | Image path/filename in uploads/ | |
| | description | TEXT | Product details | | ||
| | image | VARCHAR | Filename in uploads/ | | ||
|
|
||
| Admin credentials are stored in a separate `users` table with bcrypt-hashed passwords. |
There was a problem hiding this comment.
The admin credentials table name appears to be admins (see the query in admin/login.php), not users. Please update this line so the documentation matches the implementation/database naming.
| Admin credentials are stored in a separate `users` table with bcrypt-hashed passwords. | |
| Admin credentials are stored in a separate `admins` table with bcrypt-hashed passwords. |
| | XSS | `htmlspecialchars()` on all output | | ||
| | Session Hijacking | Session regeneration on login | | ||
| | Brute Force | Password hashing with bcrypt (`password_hash`) | | ||
| | File Upload Attacks | Type/size validation before `move_uploaded_file()` | |
There was a problem hiding this comment.
Several items in the security threat table don’t match the current implementation: there is no session ID regeneration on login, and hashing passwords is not a brute-force mitigation (there’s no rate limiting/lockout). Also, output escaping is not applied to all rendered user-controlled values (e.g., some form fields echo raw values). Please adjust the mitigations to reflect what the code actually does (or implement the missing defenses).
| | XSS | `htmlspecialchars()` on all output | | |
| | Session Hijacking | Session regeneration on login | | |
| | Brute Force | Password hashing with bcrypt (`password_hash`) | | |
| | File Upload Attacks | Type/size validation before `move_uploaded_file()` | | |
| | XSS | `htmlspecialchars()` on public product listing; some admin outputs still echo raw values | | |
| | Session Hijacking | Session-based access control via `auth_check.php`; no session ID regeneration on login | | |
| | Brute Force | Credentials stored with bcrypt (`password_hash`); no login rate limiting or lockout implemented | | |
| | File Upload Attacks | Images uploaded via `move_uploaded_file()`; additional type/size validation required | |
Changes