Skip to content
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.

Sign up for GitHub

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

Add/update SMTP auth providers via cli #18197

Merged
merged 4 commits into from
Jan 12, 2022
Merged

Add/update SMTP auth providers via cli #18197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8da8762
Add/update SMTP auth providers via cli
zpericic Jan 6, 2022
08962b9
Merge branch 'main' into smtp-auth-cli
wxiaoguang Jan 11, 2022
f89ccd6
Merge branch 'main' into smtp-auth-cli
wxiaoguang Jan 12, 2022
204d869
Merge branch 'main' into smtp-auth-cli
zeripath Jan 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
181 changes: 181 additions & 0 deletions cmd/admin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"code.gitea.io/gitea/modules/storage"
auth_service "code.gitea.io/gitea/services/auth"
"code.gitea.io/gitea/services/auth/source/oauth2"
"code.gitea.io/gitea/services/auth/source/smtp"
repo_service "code.gitea.io/gitea/services/repository"
user_service "code.gitea.io/gitea/services/user"

Expand Down Expand Up @@ -190,6 +191,8 @@ var (
cmdAuthUpdateLdapBindDn,
cmdAuthAddLdapSimpleAuth,
cmdAuthUpdateLdapSimpleAuth,
microcmdAuthAddSMTP,
microcmdAuthUpdateSMTP,
microcmdAuthList,
microcmdAuthDelete,
},
Expand Down Expand Up @@ -366,6 +369,72 @@ var (
},
},
}

smtpCLIFlags = []cli.Flag{
cli.StringFlag{
Name: "name",
Value: "",
Usage: "Application Name",
},
cli.StringFlag{
Name: "auth-type",
Value: "PLAIN",
Usage: "SMTP Authentication Type (PLAIN/LOGIN/CRAM-MD5) default PLAIN",
},
cli.StringFlag{
Name: "host",
Value: "",
Usage: "SMTP Host",
},
cli.IntFlag{
Name: "port",
Usage: "SMTP Port",
},
cli.BoolTFlag{
Name: "force-smtps",
Usage: "SMTPS is always used on port 465. Set this to force SMTPS on other ports.",
},
cli.BoolTFlag{
Name: "skip-verify",
Usage: "Skip TLS verify.",
},
cli.StringFlag{
Name: "helo-hostname",
Value: "",
Usage: "Hostname sent with HELO. Leave blank to send current hostname",
},
cli.BoolTFlag{
Name: "disable-helo",
Usage: "Disable SMTP helo.",
},
cli.StringFlag{
Name: "allowed-domains",
Value: "",
Usage: "Leave empty to allow all domains. Separate multiple domains with a comma (',')",
},
cli.BoolTFlag{
Name: "skip-local-2fa",
Usage: "Skip 2FA to log on.",
},
cli.BoolTFlag{
Name: "active",
Usage: "This Authentication Source is Activated.",
},
}

microcmdAuthAddSMTP = cli.Command{
Name: "add-smtp",
Usage: "Add new SMTP authentication source",
Action: runAddSMTP,
Flags: smtpCLIFlags,
}

microcmdAuthUpdateSMTP = cli.Command{
Name: "update-smtp",
Usage: "Update existing SMTP authentication source",
Action: runUpdateSMTP,
Flags: append(smtpCLIFlags[:1], append([]cli.Flag{idFlag}, smtpCLIFlags[1:]...)...),
}
)

func runChangePassword(c *cli.Context) error {
Expand Down Expand Up @@ -804,6 +873,118 @@ func runUpdateOauth(c *cli.Context) error {
return auth.UpdateSource(source)
}

func parseSMTPConfig(c *cli.Context, conf *smtp.Source) error {
if c.IsSet("auth-type") {
conf.Auth = c.String("auth-type")
validAuthTypes := []string{"PLAIN", "LOGIN", "CRAM-MD5"}
if !contains(validAuthTypes, strings.ToUpper(c.String("auth-type"))) {
return errors.New("Auth must be one of PLAIN/LOGIN/CRAM-MD5")
}
conf.Auth = c.String("auth-type")
}
if c.IsSet("host") {
conf.Host = c.String("host")
}
if c.IsSet("port") {
conf.Port = c.Int("port")
}
if c.IsSet("allowed-domains") {
conf.AllowedDomains = c.String("allowed-domains")
}
if c.IsSet("force-smtps") {
conf.ForceSMTPS = c.BoolT("force-smtps")
}
if c.IsSet("skip-verify") {
conf.SkipVerify = c.BoolT("skip-verify")
}
if c.IsSet("helo-hostname") {
conf.HeloHostname = c.String("helo-hostname")
}
if c.IsSet("disable-helo") {
conf.DisableHelo = c.BoolT("disable-helo")
}
if c.IsSet("skip-local-2fa") {
conf.SkipLocalTwoFA = c.BoolT("skip-local-2fa")
}
return nil
}

func runAddSMTP(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()

if err := initDB(ctx); err != nil {
return err
}

if !c.IsSet("name") || len(c.String("name")) == 0 {
return errors.New("name must be set")
}
if !c.IsSet("host") || len(c.String("host")) == 0 {
return errors.New("host must be set")
}
if !c.IsSet("port") {
return errors.New("port must be set")
}
var active = true
if c.IsSet("active") {
active = c.BoolT("active")
}

var smtpConfig smtp.Source
if err := parseSMTPConfig(c, &smtpConfig); err != nil {
return err
}

// If not set default to PLAIN
if len(smtpConfig.Auth) == 0 {
smtpConfig.Auth = "PLAIN"
}

return auth.CreateSource(&auth.Source{
Type: auth.SMTP,
Name: c.String("name"),
IsActive: active,
Cfg: &smtpConfig,
})
}

func runUpdateSMTP(c *cli.Context) error {
if !c.IsSet("id") {
return fmt.Errorf("--id flag is missing")
}

ctx, cancel := installSignals()
defer cancel()

if err := initDB(ctx); err != nil {
return err
}

source, err := auth.GetSourceByID(c.Int64("id"))
if err != nil {
return err
}

smtpConfig := source.Cfg.(*smtp.Source)

if err := parseSMTPConfig(c, smtpConfig); err != nil {
return err
}

if c.IsSet("name") {
source.Name = c.String("name")
}

if c.IsSet("active") {
source.IsActive = c.BoolT("active")
}

source.Cfg = smtpConfig

return auth.UpdateSource(source)
}

func runListAuth(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()
Expand Down
27 changes: 27 additions & 0 deletions docs/content/doc/usage/command-line.en-us.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,33 @@ Admin operations:
- `--restricted-group`: Group Claim value for restricted users. (Optional)
- Examples:
- `gitea admin auth update-oauth --id 1 --name external-github-updated`
- `add-smtp`:
- Options:
- `--name`: Application Name. Required.
- `--auth-type`: SMTP Authentication Type (PLAIN/LOGIN/CRAM-MD5). Default to PLAIN.
- `--host`: SMTP host. Required.
- `--port`: SMTP port. Required.
- `--force-smtps`: SMTPS is always used on port 465. Set this to force SMTPS on other ports.
- `--skip-verify`: Skip TLS verify.
- `--helo-hostname`: Hostname sent with HELO. Leave blank to send current hostname.
- `--disable-helo`: Disable SMTP helo.
- `--allowed-domains`: Leave empty to allow all domains. Separate multiple domains with a comma (',').
- `--skip-local-2fa`: Skip 2FA to log on.
- `--active`: This Authentication Source is Activated.
Remarks:
`--force-smtps`, `--skip-verify`, `--disable-helo`, `--skip-loca-2fs` and `--active` options can be used in form:
- `--option`, `--option=true` to enable
- `--option=false` to disable
If those options are not specified value would not be changed in `update-smtp` or would use default `false` value in `add-smtp`
- Examples:
- `gitea admin auth add-smtp --name ldap --host smtp.mydomain.org --port 587 --skip-verify --active`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document differs (--skip-verify vs --skip-verify=false):

`gitea admin auth add-smtp --name ldap --host smtp.mydomain.org --port 587 --skip-verify --active`

`gitea admin auth update-smtp -id 1 --host smtp.mydomain.org --port 587 --skip-verify=false`
`gitea admin auth update-smtp -id 1 --active=false`

ps: the document says --id instead of the -id in the example.

Since most options are shared/common, maybe we can also group these options:

- `add-smtp`:
   - add-smtp/update-smtp shared options: 
      - .....
- `update-smtp`:
    - Options:
        - `--id`: ID of source to be updated. Required.
        - other options are shared with `add-smtp`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document differs (--skip-verify vs --skip-verify=false):

`gitea admin auth add-smtp --name ldap --host smtp.mydomain.org --port 587 --skip-verify --active`

All bool options uses BoolTFlag which allows using --option=[true|false] format and --option defaults to true. If option is not specified than those option is not changed. See github.com/urfave/cli.

Many of BoolFlag options in cli should be migrated to BoolTFlag as there is no way to disable those options. See my remark in #18197 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So usually just not setting the option is the best way of indicating that the option is meant to be false but if BoolTFlag allows both then that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So usually just not setting the option is the best way of indicating that the option is meant to be false but if BoolTFlag allows both then that's fine.

Omitting option for false is fine in 'add-smtp'. But in 'update-smtp' that behavior would require specifying all enabled options on every call. With my approach you could just do gitea admin auth update-smtp --id 1 --active=false to deactivate provider without changing or specifying other options.

At first I replicated add-oauth and update-oauth approach but in runUpdateOauth they also ended with "change needed options" instead of "change all on every call". Their approach is fine for string options as you could specify --flag="" but not for bool options. As a example to implement skip-local-2fa option handling in runUpdateOauth we could:

  1. Continue using BoolFlag() and
if c.IsSet("skip-local-2fa") {
  oAuth2Config.SkipLocalTwoFA = c.Bool("skip-local-2fa")
}

which would not allow us to disable option as omitting options would just leave last value.

  1. Use BoolFlag() and
oAuth2Config.SkipLocalTwoFA = c.Bool("skip-local-2fa")

which would always set option. But omitting option would change it to false which would be contrary to approach for other options.

  1. Use BoolTFlag() and
if c.IsSet("skip-local-2fa") {
  oAuth2Config.SkipLocalTwoFA = c.BoolT("skip-local-2fa")
}

which would set option to value specified (either true or false) or leave option unchanged if omitted.

Copy link
Contributor

@wxiaoguang wxiaoguang Jan 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ps: I meant that the documents differ for add and update.

* For add: it says --skip-verify --active (and, does --skip-verify work with BoolT correctly? undefined behavior?)
* For update: it says --skip-verify=false

It confuses readers.

Copy link
Contributor

@wxiaoguang wxiaoguang Jan 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm ... the cli library does that trick, so --option is the same as --option=true.

👍 No question from my side.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Urfave cli library is really poorly documented. It took a while till I stumbled on right option.

- `update-smtp`:
- Options:
- `--id`: ID of source to be updated. Required.
- other options are shared with `add-smtp`
- Examples:
- `gitea admin auth update-smtp --id 1 --host smtp.mydomain.org --port 587 --skip-verify=false`
- `gitea admin auth update-smtp --id 1 --active=false`
- `add-ldap`: Add new LDAP (via Bind DN) authentication source
- Options:
- `--name value`: Authentication name. Required.
Expand Down