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

Jump to bottom

Auth support in frontends #3559

Merged
merged 28 commits into from
Jul 6, 2018
Merged

Auth support in frontends #3559

merged 28 commits into from
Jul 6, 2018

Conversation

jbdoumenjou
Copy link
Member

@jbdoumenjou jbdoumenjou commented Jul 4, 2018 •
edited
Loading

What does this PR do?

Authentication support in frontends for the following providers:

Motivation

Fixes #2116, #2162, #2734
Related to #1465, #3460

More

  • Added/updated tests
  • Added/updated documentation

@jbdoumenjou jbdoumenjou requested review from a team as code owners July 4, 2018 13:05
@mmatur mmatur added this to the 1.7 milestone Jul 4, 2018
ldez
ldez suggested changes Jul 4, 2018
View reviewed changes
provider/kv/kv_config.go Outdated
return basicAuth
}

//
Copy link
Contributor

Choose a reason for hiding this comment

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

can you remove this line.

provider/kv/kv_config.go Outdated

// GetAuth Create auth from labels
func (p *Provider) getAuth(rootPath string) *types.Auth {

Copy link
Contributor

Choose a reason for hiding this comment

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

can you remove this empty line.

provider/kv/kv_config_test.go Outdated
@@ -24,38 +24,188 @@ func TestProviderBuildConfiguration(t *testing.T) {
kvPairs []*store.KVPair
expected *types.Configuration
}{
//{
Copy link
Contributor

Choose a reason for hiding this comment

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

could you uncomment or remove this test.

templates/ecs.tmpl Outdated

{{ $whitelist := getWhiteList $instance.TraefikLabels }}
{{ $whitelist := getWhiteList $instance.TraefikLabels }}
Copy link
Contributor

Choose a reason for hiding this comment

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

can you restore the previous line.

templates/kv.tmpl Outdated

{{ $whitelist := getWhiteList $frontend }}
{{ $whitelist := getWhiteList $frontend }}
Copy link
Contributor

Choose a reason for hiding this comment

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

can you restore the previous line.

templates/marathon.tmpl Outdated

{{ $whitelist := getWhiteList $app.SegmentLabels }}
{{ $whitelist := getWhiteList $app.SegmentLabels }}
Copy link
Contributor

Choose a reason for hiding this comment

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

can you restore the previous line.

templates/mesos.tmpl Outdated

{{ $whitelist := getWhiteList $app.TraefikLabels }}
{{ $whitelist := getWhiteList $app.TraefikLabels }}
Copy link
Contributor

Choose a reason for hiding this comment

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

can you restore the previous line.

templates/rancher.tmpl Outdated

{{ $whitelist := getWhiteList $service.SegmentLabels }}
{{ $whitelist := getWhiteList $service.SegmentLabels }}
Copy link
Contributor

Choose a reason for hiding this comment

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

can you restore the previous line.

provider/kv/kv_config.go Outdated
TrustForwardHeader: p.getBool(false, rootPath, pathFrontendAuthForwardTrustForwardHeader),
}

//TLS configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a whitespace.

// TLS configuration

provider/label/partial.go Outdated

return auth
}
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

// GetAuth Create auth from labels
func GetAuth(labels map[string]string) *types.Auth {
	if !HasPrefix(labels, TraefikFrontendAuth) {
		return nil
	}

	auth := &types.Auth{
		HeaderField: GetStringValue(labels, TraefikFrontendAuthHeaderField, ""),
	}

	if HasPrefix(labels, TraefikFrontendAuthBasic) {
		auth.Basic = getAuthBasic(labels)
	} else if HasPrefix(labels, TraefikFrontendAuthDigest) {
		auth.Digest = getAuthDigest(labels)
	} else if HasPrefix(labels, TraefikFrontendAuthForward) {
		auth.Forward = getAuthForward(labels)
	}

	return auth
}

@jbdoumenjou jbdoumenjou requested a review from a team as a code owner July 5, 2018 07:55
mmatur
mmatur requested changes Jul 5, 2018
View reviewed changes
docs/configuration/backends/consulcatalog.md Outdated
| `<prefix>.frontend.auth.digest.users=EXPR` | Sets digest authentication to this frontend in CSV format: `User:Realm:Hash,User:Realm:Hash`. |
| `<prefix>.frontend.auth.digest.usersfile=/path/.htdigest` | Sets digest authentication with an external file; if users and usersFile are provided, both are merged, with external file contents having precedence. |
| `<prefix>.frontend.auth.forward.address=https://example.com`| The URL of the authentication server. |
| `<prefix>.frontend.auth.forward.tls.ca=/path/ca.pem` | Set the Certificate Authority (CA) for the TLS connection with the authentication server. |
Copy link
Member

Choose a reason for hiding this comment

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

Sets instead of Set

docs/configuration/backends/consulcatalog.md Outdated
| `<prefix>.frontend.auth.forward.address=https://example.com`| The URL of the authentication server. |
| `<prefix>.frontend.auth.forward.tls.ca=/path/ca.pem` | Set the Certificate Authority (CA) for the TLS connection with the authentication server. |
| `<prefix>.frontend.auth.forward.tls.caOptional=true` | Check the certificates if present but do not force to be signed by a specified Certificate Authority (CA). |
| `<prefix>.frontend.auth.forward.tls.cert=/path/server.pem` | Set the Certificate for the TLS connection with the authentication server. |
Copy link
Member

Choose a reason for hiding this comment

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

Sets instead of Set

docs/configuration/backends/consulcatalog.md Outdated
| `<prefix>.frontend.auth.forward.tls.caOptional=true` | Check the certificates if present but do not force to be signed by a specified Certificate Authority (CA). |
| `<prefix>.frontend.auth.forward.tls.cert=/path/server.pem` | Set the Certificate for the TLS connection with the authentication server. |
| `<prefix>.frontend.auth.forward.tls.insecureSkipVerify=true`| If set to true invalid SSL certificates are accepted. |
| `<prefix>.frontend.auth.forward.tls.key=/path/server.key` | Set the Certificate for the TLS connection with the authentication server. |
Copy link
Member

Choose a reason for hiding this comment

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

Sets instead of Set

docs/configuration/backends/consulcatalog.md Outdated
| `<prefix>.frontend.auth.digest.usersfile=/path/.htdigest` | Sets digest authentication with an external file; if users and usersFile are provided, both are merged, with external file contents having precedence. |
| `<prefix>.frontend.auth.forward.address=https://example.com`| The URL of the authentication server. |
| `<prefix>.frontend.auth.forward.tls.ca=/path/ca.pem` | Set the Certificate Authority (CA) for the TLS connection with the authentication server. |
| `<prefix>.frontend.auth.forward.tls.caOptional=true` | Check the certificates if present but do not force to be signed by a specified Certificate Authority (CA). |
Copy link
Member

Choose a reason for hiding this comment

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

Checks instead of Check

docs/configuration/backends/docker.md Outdated
| `traefik.frontend.auth.digest.users=EXPR` | Sets digest authentication to this frontend in CSV format: `User:Realm:Hash,User:Realm:Hash`. |
| `traefik.frontend.auth.digest.usersfile=/path/.htdigest` | Sets digest authentication with an external file; if users and usersFile are provided, both are merged, with external file contents having precedence. |
| `traefik.frontend.auth.forward.address=https://example.com`| The URL of the authentication server. |
| `traefik.frontend.auth.forward.tls.ca=/path/ca.pem` | Set the Certificate Authority (CA) for the TLS connection with the authentication server. |
Copy link
Member

Choose a reason for hiding this comment

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

Sets instead of Set

docs/configuration/backends/mesos.md Outdated
| `traefik.frontend.auth.forward.tls.caOptional=true` | Check the certificates if present but do not force to be signed by a specified Certificate Authority (CA). |
| `traefik.frontend.auth.forward.tls.cert=/path/server.pem` | Set the Certificate for the TLS connection with the authentication server. |
| `traefik.frontend.auth.forward.tls.insecureSkipVerify=true`| If set to true invalid SSL certificates are accepted. |
| `traefik.frontend.auth.forward.tls.key=/path/server.key` | Set the Certificate for the TLS connection with the authentication server. |
Copy link
Member

Choose a reason for hiding this comment

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

Sets instead of Set

docs/configuration/backends/rancher.md Outdated
| `traefik.frontend.auth.digest.users=EXPR` | Sets digest authentication to this frontend in CSV format: `User:Realm:Hash,User:Realm:Hash`. |
| `traefik.frontend.auth.digest.usersfile=/path/.htdigest` | Sets digest authentication with an external file; if users and usersFile are provided, both are merged, with external file contents having precedence. |
| `traefik.frontend.auth.forward.address=https://example.com`| The URL of the authentication server. |
| `traefik.frontend.auth.forward.tls.ca=/path/ca.pem` | Set the Certificate Authority (CA) for the TLS connection with the authentication server. |
Copy link
Member

Choose a reason for hiding this comment

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

Sets instead of Set

docs/configuration/backends/rancher.md Outdated
| `traefik.frontend.auth.digest.usersfile=/path/.htdigest` | Sets digest authentication with an external file; if users and usersFile are provided, both are merged, with external file contents having precedence. |
| `traefik.frontend.auth.forward.address=https://example.com`| The URL of the authentication server. |
| `traefik.frontend.auth.forward.tls.ca=/path/ca.pem` | Set the Certificate Authority (CA) for the TLS connection with the authentication server. |
| `traefik.frontend.auth.forward.tls.caOptional=true` | Check the certificates if present but do not force to be signed by a specified Certificate Authority (CA). |
Copy link
Member

Choose a reason for hiding this comment

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

Checks instead of Check

docs/configuration/backends/rancher.md Outdated
| `traefik.frontend.auth.forward.address=https://example.com`| The URL of the authentication server. |
| `traefik.frontend.auth.forward.tls.ca=/path/ca.pem` | Set the Certificate Authority (CA) for the TLS connection with the authentication server. |
| `traefik.frontend.auth.forward.tls.caOptional=true` | Check the certificates if present but do not force to be signed by a specified Certificate Authority (CA). |
| `traefik.frontend.auth.forward.tls.cert=/path/server.pem` | Set the Certificate for the TLS connection with the authentication server. |
Copy link
Member

Choose a reason for hiding this comment

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

Sets instead of Set

docs/configuration/backends/rancher.md Outdated
| `traefik.frontend.auth.forward.tls.caOptional=true` | Check the certificates if present but do not force to be signed by a specified Certificate Authority (CA). |
| `traefik.frontend.auth.forward.tls.cert=/path/server.pem` | Set the Certificate for the TLS connection with the authentication server. |
| `traefik.frontend.auth.forward.tls.insecureSkipVerify=true`| If set to true invalid SSL certificates are accepted. |
| `traefik.frontend.auth.forward.tls.key=/path/server.key` | Set the Certificate for the TLS connection with the authentication server. |
Copy link
Member

Choose a reason for hiding this comment

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

Sets instead of Set

mmatur
mmatur requested changes Jul 5, 2018
View reviewed changes
provider/marathon/config_test.go Outdated
"test:$apr1$H6uskkkW$IgXLP6ewTrSuBkTrqE8wj/",
"test2:$apr1$d9hr9HBB$4HxwgUir3HP4EsggP/QNo0",
Auth: &types.Auth{
//HeaderField: "X-WebAuth-User",
Copy link
Member

Choose a reason for hiding this comment

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

Why keep this comment?

provider/marathon/config_test.go Outdated
Basic: &types.Basic{
Users: []string{"test:$apr1$H6uskkkW$IgXLP6ewTrSuBkTrqE8wj/",
"test2:$apr1$d9hr9HBB$4HxwgUir3HP4EsggP/QNo0"},
//UsersFile: ".htpasswd",
Copy link
Member

Choose a reason for hiding this comment

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

Why keep this comment?

templates/kv.tmpl Outdated
{{end}}

{{if $auth.Digest }}
[frontends.{{ $frontendName }}.auth.digest]
Copy link
Member

Choose a reason for hiding this comment

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

Why there are no " for {{ $frontendName }}?

templates/kv.tmpl Outdated
{{end}}

{{if $auth.Basic }}
[frontends.{{ $frontendName }}.auth.basic]
Copy link
Member

Choose a reason for hiding this comment

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

Why there are no " for {{ $frontendName }}?

templates/kv.tmpl Outdated
trustForwardHeader = {{ $auth.Forward.TrustForwardHeader }}

{{if $auth.Forward.TLS }}
[frontends.{{ $frontendName }}.auth.forward.tls]
Copy link
Member

Choose a reason for hiding this comment

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

Why there are no " for {{ $frontendName }}?

templates/mesos.tmpl Outdated

{{if $auth.Digest }}
[frontends.frontend-{{ $frontendName }}.auth.digest]
Copy link
Member

Choose a reason for hiding this comment

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

Why there are no " for frontend-{{ $frontendName }}?

templates/rancher.tmpl Outdated
headerField = "{{ $auth.HeaderField }}"

{{if $auth.Forward }}
[frontends.frontend-{{ $frontendName }}.auth.forward]
Copy link
Member

Choose a reason for hiding this comment

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

Why there are no " for frontend-{{ $frontendName }}?

templates/rancher.tmpl Outdated
trustForwardHeader = {{ $auth.Forward.TrustForwardHeader }}

{{if $auth.Forward.TLS }}
[frontends.frontend-{{ $frontendName }}.auth.forward.tls]
Copy link
Member

Choose a reason for hiding this comment

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

Why there are no " for frontend-{{ $frontendName }}?

templates/rancher.tmpl Outdated
{{end}}

{{if $auth.Basic }}
[frontends.frontend-{{ $frontendName }}.auth.basic]
Copy link
Member

Choose a reason for hiding this comment

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

Why there are no " for frontend-{{ $frontendName }}?

templates/rancher.tmpl Outdated
{{end}}

{{if $auth.Digest }}
[frontends.frontend-{{ $frontendName }}.auth.digest]
Copy link
Member

Choose a reason for hiding this comment

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

Why there are no " for frontend-{{ $frontendName }}?

@jbdoumenjou jbdoumenjou force-pushed the feature/auth_frontend branch from e2b6aef to 543c282 Compare July 5, 2018 15:42
@mmatur
Copy link
Member

mmatur commented Jul 5, 2018 •
edited
Loading

@jbdoumenjou on your branch it is remaining this part

https://github.com/jbdoumenjou/traefik/blob/543c28296c190658b99cb864dcaa3b8051b1259e/templates/kubernetes.tmpl#L55-L57

Is it normal?

@jbdoumenjou
Copy link
Member Author

@mmatur, I hadn't worked on the k8s provider, it is another PR (#3460 ). It seems that we keep this part of the template. So, I don't know if I have to clean the k8s template.

jbdoumenjou and others added 18 commits July 6, 2018 14:37
@jbdoumenjou
Fix - fix the integration tests
e5f5c27
@jbdoumenjou
Update the docker documentation
f774c6d
@jbdoumenjou
Fix for review - typo/comment
fe1946a
@jbdoumenjou
Update the docker documentation on segments
78b2c20
@jbdoumenjou
Fix for review - fix the docker documentation
c673eb8
@jbdoumenjou
Fix for review - fix the docker documentation
13395e1
@jbdoumenjou
Fix for review - the documentation for kubernetes
c56b93b
@jbdoumenjou
Update the rancher documentation
ee99a85
@jbdoumenjou
Update the consul_catalog documentation
a1c500a
@jbdoumenjou
Update the marathon documentation
e41c486
@jbdoumenjou
Update the mesos documentation
5dbec4e
@jbdoumenjou
Update the ecs documentation
cc2af29
@jbdoumenjou
Rephrase the docker documentation
36337e3
@jbdoumenjou
Fix for review - remove useless comments on the marathon config tests
bb9a8d6
@jbdoumenjou
Fix for review - fix templates
60acd20
@jbdoumenjou
Fix for review - all the labels documentation
19933f1
@jbdoumenjou
Fix for rebase on master
5586265
@mmatur @jbdoumenjou
review: remove unused basicAuth in kubernetes
f805d21
@jbdoumenjou jbdoumenjou force-pushed the feature/auth_frontend branch from 7df06f3 to f805d21 Compare July 6, 2018 12:49
@traefiker traefiker merged commit 79bf19c into traefik:master Jul 6, 2018
@jbdoumenjou jbdoumenjou deleted the feature/auth_frontend branch July 6, 2018 14:53
@kachkaev kachkaev mentioned this pull request Aug 1, 2018
Closed
@ldez ldez mentioned this pull request Aug 1, 2018
Closed
@crimoniv crimoniv mentioned this pull request Sep 6, 2018
Closed
@XA21X XA21X mentioned this pull request Jan 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmatur mmatur mmatur approved these changes

@ldez ldez ldez approved these changes

@nmengin nmengin nmengin approved these changes

@traefiker traefiker traefiker approved these changes

Assignees
No one assigned
Labels
area/authentication area/provider/consul area/provider/consulcatalog area/provider/docker area/provider/ecs area/provider/kv area/provider/marathon area/provider/mesos area/provider/rancher kind/enhancement a new or improved feature. size/L
Projects
None yet
Milestone
1.7
Development

Successfully merging this pull request may close these issues.
5 participants
@jbdoumenjou @mmatur @ldez @nmengin @traefiker