mod_auth_gssapi: Add support for every configuration directive #2214

canth1 · 2022-02-11T19:02:18Z

This allows every configuration directive available to mod_auth_gssapi to be configured inside a directory section of a vhost.

Example

include apache::mod::auth_gssapi
apache::vhost { 'sample.example.net':
  docroot     => '/path/to/directory',
  directories => [
    { path   => '/path/to/different/dir',
      gssapi => {
        acceptor_name            => '{HOSTNAME}',
        allowed_mech             => ['krb5', 'iakerb', 'ntlmssp'],
        basic_auth               => true,
        basic_auth_mech          => ['krb5', 'iakerb', 'ntlmssp'],
        basic_ticket_timeout     => 300,
        connection_bound         => true,
        cred_store               => {
          ccache        => ['/path/to/directory'],
          client_keytab => ['/path/to/example.keytab'],
          keytab        => ['/path/to/example.keytab'],
        },
        deleg_ccache_dir         => '/path/to/directory',
        deleg_ccache_env_var     => 'KRB5CCNAME',
        deleg_ccache_perms       => {
          mode => '0600',
          uid  => 'example-user',
          gid  => 'example-group',
        },
        deleg_ccache_unique      => true,
        impersonate              => true,
         local_name              => true,
        name_attributes          => 'json',
        negotiate_once           => true,
        publish_errors           => true,
        publish_mech             => true,
        required_name_attributes =>	'auth-indicators=high',
        session_key              => 'file:/path/to/example.key',
        signal_persistent_auth   => true,
        ssl_only                 => true,
        use_s4u2_proxy           => true,
        use_sessions             => true,
      }
    },
  ],
}

The above config results in an Apache configuration of:

<Directory /path/to/directory>
    # mod_auth_gssapi configuration
    GssapiAcceptorName {HOSTNAME}
    GssapiAllowedMech krb5
    GssapiAllowedMech iakerb
    GssapiAllowedMech ntlmssp
    GssapiBasicAuth On
    GssapiBasicAuthMech krb5
    GssapiBasicAuthMech iakerb
    GssapiBasicAuthMech ntlmssp
    GssapiBasicTicketTimeout 300
    GssapiConnectionBound On
    GssapiCredStore ccache:FILE:/path/to/directory
    GssapiCredStore client_keytab:/path/to/example.keytab
    GssapiCredStore keytab:/path/to/example.keytab
    GssapiDelegCcacheDir /path/to/directory
    GssapiDelegCcacheEnvVar KRB5CCNAME
    GssapiDelegCcachePerms mode:0600 uid:example-user gid:example-group
    GssapiDelegCcacheUnique On
    GssapiImpersonate On
    GssapiLocalName On
    GssapiNameAttributes json
    GssapiNegotiateOnce On
    GssapiPublishErrors On
    GssapiPublishMech On
    GssapiRequiredNameAttributes "auth-indicators=high"
    GssapiSessionKey file:/path/to/example.key
    GssapiSignalPersistentAuth On
    GssapiSSLonly On
    GssapiUseS4U2Proxy On
    GssapiUseSessions On
</Directory>

CLAassistant · 2022-02-11T19:02:23Z

All committers have signed the CLA.

puppet-community-rangefinder · 2022-02-11T19:02:26Z

apache::vhost is a type

Breaking changes to this file WILL impact these 129 modules (exact match):

Breaking changes to this file MAY impact these 34 modules (near match):

This module is declared in 172 of 578 indexed public `Puppetfiles`.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

david22swan

@canth1 Hey, sorry for the wait on review.
Anyway, while this look's like a great change and an excellent addition of functionality it is causing some unit test failures seeming to originate from the new acceptor_name value.
If you could get this solved I would be happy to merge, I've included an example of the error below:

Failures:

  1) apache::vhost os-independent items on oraclelinux-6-x86_64  set everything! is expected to compile into a catalogue without dependency cycles
     Failure/Error: acceptor_name            => '{HOSTNAME}',

     NameError:
       undefined local variable or method `acceptor_name' for #<RSpec::ExampleGroups::ApacheVhost::OsIndependentItems::OnOraclelinux6X8664::SetEverything:0x00005576c6b33570>
     # ./spec/defines/vhost_spec.rb:255:in `block (6 levels) in <top (required)>'
     # ./spec/defines/vhost_spec.rb:531:in `block (6 levels) in <top (required)>'
     # ./vendor/bundle/ruby/2.5.0/bin/rspec:23:in `load'
     # ./vendor/bundle/ruby/2.5.0/bin/rspec:23:in `<top (required)>'
     # ./vendor/bundle/ruby/2.5.0/bin/bundle:23:in `load'
     # ./vendor/bundle/ruby/2.5.0/bin/bundle:23:in `<main>'

david22swan · 2022-05-10T14:28:21Z

@canth1 Sorry to come back but while reviewing your pr I noticed that a similar change had been pushed up by another contributor that contains part of the functionality that you yourself have added. Just felt I should alert you as depending on which PR is merged first you may need to rebase your work.
Have placed a similar comment on the other PR, linked below:

Allow additional settings for GSSAPI in Vhost #2215

david22swan · 2022-05-10T14:38:27Z

In relation to the comment above, have discovered two more PRs that are adding similar functionality to this, one of which was in a good enough state to be merged:

mod_auth_gssapi: Add support for GssapiBasicAuth. #2212

And another which will require some more work:

Added in basic auth and no-negotiate options #2204

canth1 · 2022-05-10T17:05:19Z

Thanks for the update on this. I'll work on getting those issues fixed. As far as the other pull requests, I would argue that you should use this one as it is a complete implementation of every mod_auth_gssapi configuration option. From what I've seen, the other pull requests only add a few additional options, leaving the functionality incomplete.

canth1 · 2022-05-10T17:38:01Z

Looking at this closer, is the issue that the { and } aren't escaped correctly in the vhost_spec.rb file?

https://github.com/puppetlabs/puppetlabs-apache/pull/2214/files#diff-e1ca2aff131effcf3317f6b0fb468368163f59c5cb4c9598f15729bfe288646aR976

david22swan · 2022-05-11T10:00:26Z

I don't think so, from the way the error is presented it seem's to come from where the acceptor_name is being set.

david22swan · 2022-05-11T10:05:33Z

Reading through the errorlogs see a lot of:

          undefined local variable or method `acceptor_name' for #<RSpec::ExampleGroups::ApacheVhost::OsIndependentItems::OnRedhat6X8664::SetEverything:0x00005576d5d9ed78>

david22swan · 2022-06-06T09:34:55Z

@canth1 Is this still something you plan to work on?

canth1 · 2022-06-06T15:31:52Z

@canth1 Is this still something you plan to work on?

I really do, but from everything I've looked at I can't figure out what's throwing that error. Is it possible that it's a false positive?

david22swan · 2022-06-07T08:59:54Z

@canth1 Took a look at your failure and found what was causing it, basically you need to put your test values in strings, they don't mock correctly if just passed in naked, i.e. include them as:

'acceptor_name'            => '{HOSTNAME}',

Sorry it took me so long to remember the test limitation, but if you fix this and rebase then your pr to resolve the conflicts should be good to merge.

canth1 · 2022-06-07T16:01:46Z

Ok, I've updated the vhost_spec.rb and the issue should be fixed. Let me know if you need anything else from me!

david22swan · 2022-06-07T16:32:41Z

@canth1 Just one more change sorry.
You need to resolve the conflicts with the main branch, basically some alterations where made to it following the creation of your branch that now conflict with your own changes.
So you need to either rebase your changes onto the main branch, or hit the Github Resolve conflicts button and go through them manually.
It shouldn't take long.

canth1 · 2022-06-07T19:07:41Z

@canth1 Just one more change sorry. You need to resolve the conflicts with the main branch, basically some alterations where made to it following the creation of your branch that now conflict with your own changes. So you need to either rebase your changes onto the main branch, or hit the Github Resolve conflicts button and go through them manually. It shouldn't take long.

Ok, I just fixed those conflicts.

david22swan

LGTM

david22swan · 2022-06-08T12:24:04Z

@canth1 Ok that's that then, sorry for asking for so many changes but it all looks good now so I'm gonna go ahead and merge.
Thanks for putting in the work :)

canth1 · 2022-06-08T13:32:49Z

@canth1 Ok that's that then, sorry for asking for so many changes but it all looks good now so I'm gonna go ahead and merge. Thanks for putting in the work :)

Of course. I use this Puppet module extensively, so I'm glad that I'm able to contribute back to it. Thanks again!

mod_auth_gssapi: Add support for every configuration directive

58217d9

canth1 requested a review from a team as a code owner February 11, 2022 19:02

olifre mentioned this pull request Mar 1, 2022

mod_auth_gssapi: Add support for GssapiBasicAuth. #2212

Merged

chelnak added the community label Mar 9, 2022

bastelfreak previously approved these changes Mar 18, 2022

View reviewed changes

david22swan requested changes May 10, 2022

View reviewed changes

david22swan mentioned this pull request May 10, 2022

Allow additional settings for GSSAPI in Vhost #2215

Merged

-Fix unit test errors

7f797cc

canth1 dismissed bastelfreak’s stale review via 7f797cc June 7, 2022 15:59

Merge branch 'main' into master

e60af06

david22swan added the feature label Jun 8, 2022

david22swan approved these changes Jun 8, 2022

View reviewed changes

david22swan merged commit 03df30e into puppetlabs:main Jun 8, 2022

david22swan mentioned this pull request Jun 8, 2022

Added in basic auth and no-negotiate options #2204

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mod_auth_gssapi: Add support for every configuration directive #2214

mod_auth_gssapi: Add support for every configuration directive #2214

canth1 commented Feb 11, 2022

CLAassistant commented Feb 11, 2022 •

edited

Loading

puppet-community-rangefinder bot commented Feb 11, 2022

david22swan left a comment

david22swan commented May 10, 2022

david22swan commented May 10, 2022 •

edited

Loading

canth1 commented May 10, 2022

canth1 commented May 10, 2022

david22swan commented May 11, 2022

david22swan commented May 11, 2022

david22swan commented Jun 6, 2022

canth1 commented Jun 6, 2022

david22swan commented Jun 7, 2022 •

edited

Loading

canth1 commented Jun 7, 2022

david22swan commented Jun 7, 2022

canth1 commented Jun 7, 2022

david22swan left a comment

david22swan commented Jun 8, 2022

canth1 commented Jun 8, 2022

mod_auth_gssapi: Add support for every configuration directive #2214

mod_auth_gssapi: Add support for every configuration directive #2214

Conversation

canth1 commented Feb 11, 2022

CLAassistant commented Feb 11, 2022 • edited Loading

puppet-community-rangefinder bot commented Feb 11, 2022

apache::vhost is a type

This module is declared in 172 of 578 indexed public Puppetfiles.

david22swan left a comment

Choose a reason for hiding this comment

david22swan commented May 10, 2022

david22swan commented May 10, 2022 • edited Loading

canth1 commented May 10, 2022

canth1 commented May 10, 2022

david22swan commented May 11, 2022

david22swan commented May 11, 2022

david22swan commented Jun 6, 2022

canth1 commented Jun 6, 2022

david22swan commented Jun 7, 2022 • edited Loading

canth1 commented Jun 7, 2022

david22swan commented Jun 7, 2022

canth1 commented Jun 7, 2022

david22swan left a comment

Choose a reason for hiding this comment

david22swan commented Jun 8, 2022

canth1 commented Jun 8, 2022

CLAassistant commented Feb 11, 2022 •

edited

Loading

This module is declared in 172 of 578 indexed public `Puppetfiles`.

david22swan commented May 10, 2022 •

edited

Loading

david22swan commented Jun 7, 2022 •

edited

Loading