-
Notifications
You must be signed in to change notification settings - Fork 1
feat(otp): replace inline OTPaaS calls with provider abstraction
#181
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: main
Are you sure you want to change the base?
Changes from 7 commits
255bb87
1b8ea47
c5b3ab8
d00dfa8
5591d97
c8ef494
3d6ce9b
4719ef7
5a96d88
f45906b
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 |
|---|---|---|
|
|
@@ -10,22 +10,30 @@ import ( | |
| ) | ||
|
|
||
| type Environment string | ||
| type Provider string | ||
|
|
||
| const ( | ||
| EnvironmentDevelopment Environment = "development" | ||
| EnvironmentProduction Environment = "production" | ||
|
|
||
| ProviderOTPaaS Provider = "otpaas" | ||
| ProviderMock Provider = "mock" | ||
| ) | ||
|
|
||
| // Config is the main configuration for the application. | ||
| type Config struct { | ||
| Environment Environment `dotenv:"TW_ENV"` | ||
| LogLevel slog.Level `dotenv:"TW_LOG_LEVEL"` | ||
|
|
||
| OTPProvider Provider `dotenv:"TW_OTP_PROVIDER"` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BOTP service only focusing on OTP operations so the configurations do not need to be nested. TW will have a lot of different kind of configurations so I think we should nest OTP related configurations into a struct. For example: type Config struct {
...
OTP OTPConfig `dotenv:",squash"`
}
type OTPConfig struct {
Provider OTPProvider `dotenv:"TW_OTP_PROVIDER"`
AllowedEmailDomains []string `dotenv:"TW_OTP_ALLOWED_EMAIL_DOMAINS"`
OTPaaS OTPaaSConfig `dotenv:",squash"`
Mock MockConfig `dotenv:",squash"`
}Then we add comments to // OTPaaSConfig is the configuration for the OTPaaS OTP provider.
type OTPaaSConfig struct {
...
}
// MockConfig is the configuration for the mock OTP provider.
type MockConfig struct {
...
}WDYT? |
||
| AllowedEmailDomains []string `dotenv:"TW_ALLOWED_EMAIL_DOMAINS"` | ||
|
|
||
| ViteDevServerURL *url.URL `dotenv:"TW_VITE_DEV_SERVER_URL"` | ||
| BundleDirectory string `dotenv:"TW_BUNDLE_DIRECTORY"` | ||
|
|
||
| Server ServerConfig `dotenv:",squash"` | ||
| OTPaaS OTPaaSConfig `dotenv:",squash"` | ||
| Mock MockConfig `dotenv:",squash"` | ||
| } | ||
|
|
||
| // ServerConfig represents the configuration for the HTTP server. | ||
|
|
@@ -47,12 +55,19 @@ type OTPaaSConfig struct { | |
| Timeout time.Duration `dotenv:"TW_OTPAAS_TIMEOUT"` | ||
| } | ||
|
|
||
| type MockConfig struct { | ||
| AllowedEmails []string `dotenv:"TW_MOCK_ALLOWED_EMAILS"` | ||
| } | ||
|
|
||
| // Default returns the default configuration for the application. | ||
| func Default() *Config { | ||
| return &Config{ | ||
| Environment: EnvironmentDevelopment, | ||
| LogLevel: slog.LevelInfo, | ||
|
|
||
| OTPProvider: ProviderOTPaaS, | ||
| AllowedEmailDomains: []string{"@schools.gov.sg"}, | ||
|
|
||
| ViteDevServerURL: must(url.Parse("http://localhost:5173")), | ||
| BundleDirectory: "dist", | ||
|
|
||
|
|
@@ -72,6 +87,9 @@ func Default() *Config { | |
| Secret: "", | ||
| Timeout: 10 * time.Second, | ||
| }, | ||
| Mock: MockConfig{ | ||
| AllowedEmails: nil, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -82,6 +100,12 @@ func (cfg *Config) Validate() error { | |
| errs = append(errs, fmt.Errorf("TW_ENV must be one of %q or %q; got %q", EnvironmentDevelopment, EnvironmentProduction, cfg.Environment)) | ||
| } | ||
|
|
||
| if len(cfg.AllowedEmailDomains) == 0 { | ||
| errs = append(errs, errors.New("TW_ALLOWED_EMAIL_DOMAINS is required")) | ||
| } | ||
|
|
||
| errs = append(errs, cfg.Server.validate()) | ||
|
|
||
| switch cfg.Environment { | ||
| case EnvironmentDevelopment: | ||
| if cfg.ViteDevServerURL.Scheme != "http" && cfg.ViteDevServerURL.Scheme != "https" { | ||
|
|
@@ -100,7 +124,16 @@ func (cfg *Config) Validate() error { | |
| } | ||
| } | ||
|
|
||
| return errors.Join(append(errs, cfg.Server.validate(), cfg.OTPaaS.validate())...) | ||
| switch cfg.OTPProvider { | ||
| case ProviderOTPaaS: | ||
| errs = append(errs, cfg.OTPaaS.validate()) | ||
| case ProviderMock: | ||
| errs = append(errs, cfg.Mock.validate()) | ||
| default: | ||
| errs = append(errs, fmt.Errorf("TW_OTP_PROVIDER must be one of %q or %q; got %q", ProviderOTPaaS, ProviderMock, cfg.OTPProvider)) | ||
| } | ||
|
|
||
| return errors.Join(errs...) | ||
| } | ||
|
|
||
| func (c ServerConfig) validate() error { | ||
|
|
@@ -147,6 +180,16 @@ func (c OTPaaSConfig) validate() error { | |
| return errors.Join(errs...) | ||
| } | ||
|
|
||
| func (c MockConfig) validate() error { | ||
| var errs []error | ||
|
|
||
| if len(c.AllowedEmails) == 0 { | ||
| errs = append(errs, errors.New("TW_MOCK_ALLOWED_EMAILS is required")) | ||
| } | ||
|
|
||
| return errors.Join(errs...) | ||
| } | ||
|
|
||
| func must[T any](value T, err error) T { | ||
| if err != nil { | ||
| panic(err) | ||
|
|
||
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.
I think we should prefix with
OTPbecause there will be a lot of different provider services in the future.