-
Notifications
You must be signed in to change notification settings - Fork 11
Add plugin functionality and dummy plugin for testing #431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: bl4ze/dev
Are you sure you want to change the base?
Changes from all commits
4b94b96
343fc14
4ae61cd
fd2e11c
4ce2585
aa39819
8cf4896
bee98b7
10b02bc
e70ed01
eaacea0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,49 @@ | ||||||||||
| package plugins | ||||||||||
|
|
||||||||||
| import ( | ||||||||||
| "github.com/sdslabs/beastv4/core/config" | ||||||||||
| log "github.com/sirupsen/logrus" | ||||||||||
|
|
||||||||||
| "github.com/gin-gonic/gin" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| type Plugin interface { | ||||||||||
| Name() string | ||||||||||
| Description() string | ||||||||||
| Init(*gin.Engine) error | ||||||||||
| } | ||||||||||
|
|
||||||||||
| //All plugins will be initialized with the gin engine, at startup | ||||||||||
| //Logging twice, once when we register and once when we initialize | ||||||||||
|
Comment on lines
+16
to
+17
|
||||||||||
| //All plugins will be initialized with the gin engine, at startup | |
| //Logging twice, once when we register and once when we initialize | |
| // All plugins will be initialized with the gin engine at startup. | |
| // Logging twice: once when we register and once when we initialize. |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in comment: "Intializing" should be "Initializing".
| log.Infof("Intializing plugin: %s", p.Name()) | |
| log.Infof("Initializing plugin: %s", p.Name()) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||
| package dummy | ||||||
|
|
||||||
| import ( | ||||||
| "net/http" | ||||||
|
|
||||||
| "github.com/gin-gonic/gin" | ||||||
| "github.com/sdslabs/beastv4/plugins" | ||||||
| ) | ||||||
|
|
||||||
| type DummyPlugin struct{} | ||||||
|
|
||||||
| func (p *DummyPlugin) Name() string { | ||||||
| return "DummyPlugin" | ||||||
| } | ||||||
|
|
||||||
| func (p *DummyPlugin) Description() string { | ||||||
| return "A dummy plugin to do the testing of plugin system" | ||||||
| } | ||||||
|
|
||||||
| func (p *DummyPlugin) Init(router *gin.Engine) error { | ||||||
| router.GET("api/plugins/dummy", func(context *gin.Context) { | ||||||
|
||||||
| router.GET("api/plugins/dummy", func(context *gin.Context) { | |
| router.GET("/api/plugins/dummy", func(context *gin.Context) { |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,169 @@ | ||||||
| package emailverify | ||||||
|
|
||||||
| import ( | ||||||
| "errors" | ||||||
| "net/http" | ||||||
| "strings" | ||||||
|
|
||||||
| log "github.com/sirupsen/logrus" | ||||||
|
|
||||||
| "github.com/gin-gonic/gin" | ||||||
| "github.com/jinzhu/gorm" | ||||||
| "github.com/sdslabs/beastv4/core" | ||||||
| "github.com/sdslabs/beastv4/core/config" | ||||||
| "github.com/sdslabs/beastv4/core/database" | ||||||
| "github.com/sdslabs/beastv4/pkg/auth" | ||||||
| "github.com/sdslabs/beastv4/plugins" | ||||||
| ) | ||||||
|
|
||||||
| type HTTPPlainResp struct { | ||||||
| Message string `json:"message" example:"Messsage in response to your request"` | ||||||
|
||||||
| Message string `json:"message" example:"Messsage in response to your request"` | |
| Message string `json:"message" example:"Message in response to your request"` |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in the example string: "veifying" should be "verifying".
| Error string `json:"error" example:"Error occured while veifying the challenge."` | |
| Error string `json:"error" example:"Error occured while verifying the challenge."` |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The response types HTTPPlainResp, HTTPPlainMapResp, and HTTPErrorResp are duplicated from api/response.go (lines 7-17). This creates unnecessary code duplication and inconsistency risk.
Import and use the response types from the api package instead of redefining them.
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The email validation logic using strings.Split is too simplistic and doesn't properly validate email format. It will accept malformed emails like "user@@domain.com" or "user@" as long as there are exactly 2 parts when split by "@". Additionally, it doesn't handle edge cases like emails with plus addressing (e.g., "user+tag@domain.com") or multiple @ symbols correctly.
Consider using a more robust email validation approach, such as the net/mail package's ParseAddress function, before extracting the domain for validation.
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using log.Errorf with "WARNING:" prefix is inconsistent. Since this is a warning, use log.Warnf instead to maintain proper log level semantics and consistency with the existing codebase (line 246 in api/auth.go uses log.Printf for the same warning).
| log.Errorf("WARNING: SMTP not configured") | |
| log.Warnf("SMTP not configured") |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entire registration handler logic (lines 60-145) is duplicated from api/auth.go. This creates a significant maintainability problem because any bug fixes or updates to the registration logic must be made in two places. Additionally, the response types (HTTPPlainResp, HTTPPlainMapResp, HTTPErrorResp) are redefined on lines 19-29, duplicating the types already defined in api/response.go.
Consider refactoring the original register function in api/auth.go to accept an optional email validation function as a parameter, then call that function from the plugin. This would allow the plugin to inject its domain validation logic without duplicating the entire registration flow.
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The middleware approach unconditionally intercepts and aborts all POST requests to /auth/register when the plugin is enabled, completely replacing the original registration handler. This is problematic because:
- It bypasses the normal route registration system
- Makes the original /auth/register route (line 40 in api/router.go) unreachable when the plugin is enabled
- Creates an all-or-nothing scenario where the original handler cannot execute
A better approach would be to add the email domain validation as a check within the middleware but then call c.Next() to allow the original handler to process the request, or use a pre-handler hook pattern. Alternatively, refactor the registration logic to be composable so plugins can inject validation steps without duplicating the entire handler.
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,30 @@ | ||||||||||||||
| #!/bin/bash | ||||||||||||||
|
|
||||||||||||||
| URL="http://localhost:5005/auth/register" | ||||||||||||||
|
|
||||||||||||||
| echo "---- Testing Allowed Domain ----" | ||||||||||||||
| response_allowed=$(curl -s -o /dev/null -w "%{http_code}" -X POST $URL \ | ||||||||||||||
| -F "name=Neptune" \ | ||||||||||||||
| -F "username=neptune" \ | ||||||||||||||
| -F "password=romangod123" \ | ||||||||||||||
| -F "email=neptune@example.com") | ||||||||||||||
|
|
||||||||||||||
| if [ "$response_allowed" -eq 200 ]; then | ||||||||||||||
| echo "✅ Test Passed-Allowed Domain" | ||||||||||||||
| else | ||||||||||||||
| echo "❌ Test Failed-Allowed Domain (Got HTTP $response_allowed)" | ||||||||||||||
|
Comment on lines
+13
to
+15
|
||||||||||||||
| echo "✅ Test Passed-Allowed Domain" | |
| else | |
| echo "❌ Test Failed-Allowed Domain (Got HTTP $response_allowed)" | |
| echo "✅ Test Passed - Allowed Domain" | |
| else | |
| echo "❌ Test Failed - Allowed Domain (Got HTTP $response_allowed)" |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test script expects HTTP 200 for allowed domains, but based on the plugin implementation, when email domain validation passes and the user is successfully created, the response will be HTTP 200. However, if the same username is used in subsequent test runs, the CreateUserEntry will fail due to duplicate username constraints, causing the test to fail on repeated runs.
The test should either use unique usernames for each run (e.g., by appending a timestamp) or clean up created users after the test, to ensure the test is idempotent and can be run multiple times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space in comment. Should be "# Allowed email ids for the Email Verify plugin" (space after "#").