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.

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

refactor to go pkg structure and add rb search #7

Merged
merged 9 commits into from Mar 5, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -122,3 +122,4 @@ Temporary Items
*.ldif
user_vhost_list.conf
userVhost
rb-ldap
21 changes: 14 additions & 7 deletions README.md
@@ -1,19 +1,26 @@
# userVhost
# rb-ldap
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps repo rename would also be suiting then. But that's more of a question to @redbrick (o however you tag CMT now)

Copy link
Member Author

Choose a reason for hiding this comment

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

i did specify a repo rename above

Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies.


Script to query ldap for user info to generate apache template conf for user
vhosts.
[![Go Report Card](https://goreportcard.com/badge/github.com/redbrick/rbldap)](https://goreportcard.com/report/github.com/redbrick/rbldap)

Script to interact with Redbrick LDAP.

* query ldap for user info to generate apache template conf for user vhosts.
* Search for users in ldap

## Installation

```console
go get github.com/redbrick/userVhost
go get github.com/redbrick/rbldap/cmd/rb-ldap
```

## Run

```console
userVhost ./ldap.secret
rb-ldap
```

The conf will be output to the current dir. Run `userVhost -h` to get a list of
flags
Run `rb-ldap -h` to get a list of flags and commands.

## Notes

The conf from `rb-ldag g` will be output to the current dir.
129 changes: 129 additions & 0 deletions cmd/rb-ldap/main.go
@@ -0,0 +1,129 @@
package main

import (
"fmt"
"os"

"github.com/redbrick/rbldap/internal/pkg/rbldap"
"github.com/urfave/cli"
)

func main() {
app := cli.NewApp()
app.Name = "rb-ldap"
app.Usage = "Command line interface for Redbrick Ldap"
app.ArgsUsage = ""
app.HideVersion = true
app.EnableBashCompletion = true

app.Flags = []cli.Flag{
cli.StringFlag{
Name: "user, u",
Value: "cn=root,ou=ldap,o=redbrick",
Usage: "ldap user, used for authentication",
},

cli.StringFlag{
Name: "dcu-user",
Value: "CN=rblookup,OU=Service Accounts,DC=ad,DC=dcu,DC=ie",
Usage: "Active Directory user for DCU, used for authentication",
},

cli.StringFlag{
Name: "host",
Value: "ldap.internal",
Usage: "ldap host to query",
},

cli.StringFlag{
Name: "dcu-host",
Value: "ad.dcu.ie",
Usage: "DCU Active Directory host to query",
},

cli.IntFlag{
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to have one flag for host:port both for RB and DCU

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not a fan of a single flag as host and port won't change often and its more likely for just one too change then both. aswell in the library, they are handled separately

Copy link
Member Author

Choose a reason for hiding this comment

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

I think @CSmartt input on this would be good

Copy link
Contributor

Choose a reason for hiding this comment

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

ldap.Dial expect them combined, and they are concatenated together here: https://github.com/redbrick/userVhost/pull/7/files#diff-5bb0827606093060012c612e6c4acadbR20

Copy link
Member Author

Choose a reason for hiding this comment

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

i still would rather have a separate flag for user and port

Copy link

Choose a reason for hiding this comment

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

+1 for separation, far more likely that one will change rather than both, if at all. Worth remembering also that we're not writing a generic LDAP tool here, so while I can see the case for a combined host:port flag, I think for clarity at our end we should keep them apart.

Name: "port, p",
Value: 389,
Usage: "Port for ldap host",
},

cli.IntFlag{
Name: "dcu-port",
Value: 389,
Usage: "Port for DCU Active Directory host",
},

cli.StringFlag{
Name: "password",
Usage: "password for the ldap server",
FilePath: "/etc/ldap.secret",
},

cli.StringFlag{
Name: "dcu-password",
Usage: "password for the DCU ldap server",
FilePath: "/etc/dcu_ldap.secret",
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Even another idea is to combine all those flags into user:pass@host:port. I feel like url.URL would help with parsing. It makes the flags less awkward to use.

Copy link
Member Author

Choose a reason for hiding this comment

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

for the squashed syntax again see above.
explain url.URL

Copy link
Contributor

Choose a reason for hiding this comment

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

See https://play.golang.org/p/KAornm0cci1

You let the flag be parsed by url.Parse`. I understand the only problematic bit would be the password, which is understandably another CLI flag loading the password from file.

Edit: The password is optional so it can be omited: https://golang.org/pkg/net/url/#Userinfo.Password

Copy link
Member Author

Choose a reason for hiding this comment

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

id MUCH rather keep using the filepath for a password file rather then a password

Copy link
Member Author

Choose a reason for hiding this comment

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

that makes changing the variables seem pretty annoying

Copy link
Contributor

Choose a reason for hiding this comment

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

Even if passwords are loaded from file, once we have sane defaults, how often would you actually need to specify it yourself?

Keeping password separate we go down from 8 flags total for RB and DCU LDAP, to just 4 (user@rb:port, user@dcu:port, rb-password, dcu-password). Honestly I have never encountered decent application which forces you to change 4 different flags to log in (and I am not counting mysql as a good example).

But this is just my opinion, I think settling this rests on cmt hands :)

Copy link
Member Author

Choose a reason for hiding this comment

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

admins, not cmt
you aren't likely to be using any flags though the default will work out of the box the flags are only for if something changes

Copy link

Choose a reason for hiding this comment

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

Admin here, hi. Path to file is how this is happening, non-negotiable, I'd really prefer any user couldn't run a ps and see our pass being passed as an arg.


cli.BoolFlag{
Name: "dry-run",
Usage: "output to console rather then file",
},
}
app.Commands = []cli.Command{
{
Name: "generate",
Aliases: []string{"g"},
Usage: "generate list for uservhost macro",
Flags: []cli.Flag{
cli.StringFlag{
Name: "conf, c",
Value: "./user_vhost_list.conf",
Usage: "Output configuration `FILE`",
},
},
Action: rbldap.Generate,
},
{
Name: "search",
Aliases: []string{"s"},
Usage: "Search ldap for user",
Flags: []cli.Flag{
cli.StringFlag{
Name: "mail, altmail",
Value: "*",
Usage: "User email",
},
cli.StringFlag{
Name: "user, u, uid, nick, username",
Value: "*",
Usage: "User username",
},
cli.StringFlag{
Name: "id",
Value: "*",
Usage: "DCU id Number",
},
cli.StringFlag{
Name: "name, fullname",
Value: "*",
Usage: "User's fullname",
},
cli.BoolFlag{
Name: "newbie, noob",
Usage: "filter for new users",
},
cli.BoolFlag{
Name: "dcu, DCU",
Usage: "Query DCU Active Directory",
},
},
Action: rbldap.Search,
},
}

if err := app.Run(os.Args); err != nil {
fmt.Fprintf(os.Stderr, "error: %v\n", err)
os.Exit(1)
}
}
36 changes: 36 additions & 0 deletions internal/pkg/rbldap/generate.go
@@ -0,0 +1,36 @@
package rbldap

import (
"fmt"
"os"
"strings"

"github.com/urfave/cli"
)

// Generate takes cli context and generates user vhost for rbuser
func Generate(ctx *cli.Context) error {
l, err := newRbLdap(ctx)
if err != nil {
return err
}
vhosts, err := l.Generate()
if err != nil {
return err
}
if ctx.Bool("dry-run") {
fmt.Print(strings.Join(vhosts, "\n"))
return nil
}
file, err := os.Create(ctx.String("conf"))
if err != nil {
return err
}
defer file.Close()
n, err := file.WriteString(strings.Join(vhosts, "\n"))
if err != nil {
return err
}
fmt.Printf("wrote %d bytes %s\n", n, ctx.String("conf"))
return file.Sync()
}
43 changes: 43 additions & 0 deletions internal/pkg/rbldap/ldap.go
@@ -0,0 +1,43 @@
package rbldap

import (
"fmt"
"os"

"github.com/redbrick/rbldap/pkg/rbuser"
"github.com/urfave/cli"
)

func newRbLdap(ctx *cli.Context) (*rbuser.RbLdap, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add a TODO to not use cli.Context, in the future, seeing the maintenance track records of redbrick projects a TODO will help to show what is wrong, in case aliens abduct you and some poor admin takes care of it in the future.

This is also a prefect example where passing in url.URL would make much more sense ;)

if ctx.NArg() != 0 {
fmt.Fprintf(os.Stderr, "\n")
return &rbuser.RbLdap{}, fmt.Errorf("Missing required arguments")
}

conf := &rbuser.LdapConf{
User: ctx.String("user"),
Password: ctx.String("password"),
Host: ctx.String("host"),
Port: ctx.Int("port"),
}
rb := rbuser.RbLdap{conf}
rb.Connect()
return &rb, nil
}

func newDcuLdap(ctx *cli.Context) (*rbuser.DcuLdap, error) {
if ctx.NArg() != 0 {
fmt.Fprintf(os.Stderr, "\n")
return &rbuser.DcuLdap{}, fmt.Errorf("Missing required arguments")
}

conf := &rbuser.LdapConf{
User: ctx.String("dcu-user"),
Password: ctx.String("dcu-password"),
Host: ctx.String("dcu-host"),
Port: ctx.Int("dcu-port"),
}
dcu := rbuser.DcuLdap{conf}
dcu.Connect()
return &dcu, nil
}
74 changes: 74 additions & 0 deletions internal/pkg/rbldap/search.go
@@ -0,0 +1,74 @@
package rbldap

import (
"bytes"
"fmt"
"regexp"

"github.com/urfave/cli"
)

// Search command for cli app
func Search(ctx *cli.Context) error {
mail := filter("altmail", ctx.String("mail"))
id := filter("id", ctx.String("id"))
var re = regexp.MustCompile(`\ `)
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason why this uses var instead of :=? I have nothing against one method or another unless they are consistent or there is a reason.

name := re.ReplaceAllString(ctx.String("name"), `*$1*$2*`)
if ctx.Bool("dcu") {
dcu, err := newDcuLdap(ctx)
if err != nil {
return err
}
user, searchErr := dcu.Search(filterAnd(
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case overwriting err will have no side effects. searchErr -> err

filter("displayName", name),
filter("cn", ctx.String("user")),
id),
)
if searchErr != nil {
return searchErr
}
return user.PrettyPrint()
}
rb, err := newRbLdap(ctx)
if err != nil {
return err
}
noob := ""
if ctx.Bool("noob") {
noob = "(newbie=TRUE)"
}
user, searchErr := rb.Search(filterAnd(
filter("cn", name),
filterOr(
filter("uid", ctx.String("user")),
filter("gecos", ctx.String("user")),
), id, mail, noob),
)
if searchErr != nil {
return searchErr
}
return user.PrettyPrint()
}

func filter(key, search string) string {
return fmt.Sprintf("(%s=%s)", key, search)
}

func filterAnd(args ...string) string {
return filterJoin("&", args)
}

func filterOr(args ...string) string {
return filterJoin("|", args)
}

func filterJoin(join string, args []string) string {
var buffer bytes.Buffer
buffer.WriteString("(")
buffer.WriteString(join)
for _, filter := range args {
buffer.WriteString(filter)
}
buffer.WriteString(")")
return buffer.String()
}
9 changes: 0 additions & 9 deletions ldap.go

This file was deleted.