Puppet 3.8.7 causes issues with allowed_clock_drift values

[ananace]

The way that YAML generation is done in the gitlab.rb template runs into a big flaw when it comes to Omniauth - SAML data in particular;

A manifest like;

# ...
  class { '::gitlab':
    gitlab_rails => {
      'omniauth_enabled'                                    => $_saml_enabled,
      'omniauth_providers'                                  => [
        {
          'name'  => 'saml',
          'label' => '<Company ID>',
          'args'  => {
            'allowed_clock_drift'            => 5,
            'assertion_customer_service_url' => "${external_url}/users/auth/saml/callback",
            'idp_cert_fingerprint'           => '00:01:02:03:04:05:06:07:08:09:0A:0B:0C:0D:0E:0F:10:11:12:13',
            'idp_sso_target_url'             => '<ADFS endpoint>',
            'issuer'                         => $external_url,
            'name_identifier_format'         => 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent',
          },
        },
      ],
    },
  }

Will generate ruby code alike the following:

# gitlab.rb

#...
gitlab_rails["omniauth_providers"] => [
  {
    "args" => {
      "allowed_clock_drift" => "5",
      "assertion_consumer_service_url" => "<external url>/users/auth/saml/callback",
      "idp_cert_fingerprint" => "00:01:02:03:04:05:06:07:08:09:0A:0B:0C:0D:0E:0F:10:11:12:13",
      "idp_sso_target_url" => "<ADFS endpoint>",
      "issuer" => "<external url>",
      "name_identifier_format" => "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"
    },
    "label" => "<Company ID>",
    "name" => "saml"
  }
]

Causing OmniAuth SAML to fail its authentication attempt when trying to use the allowed_clock_drift string value as a numeral.

Reading into the compiled catalog reveals that Puppet is actually sending the value as a string literal. In fact, all numerals are strings in the catalog.

One possible way to prevent this could be to hard-code certain keys as guaranteed plain numerals in the template, might also not be a problem with newer Puppet versions.

[rwuest]

With my pull Request #109 the problem of the numerals as strings will be fixed. we had similar issues with the rack_attack_basic_auth settings.

[ananace]

Worth noting is that the problem no longer exists on Puppet 4.x and upwards, correct value types are passed into the template from that point on.

[LongLiveCHIEF]

@Ace13 can you see if the 1.13.3 release resolved this?

[ananace]

Not easily, the entire deployment is running Puppet 4.6+ by now, with auto-upgrading in place as well.

[LongLiveCHIEF]

Perhaps we should add a PR that has a test for this. The changes made by @rwuest and @jrwesolo have combined to fix integer encoding into the gitlab.rb. file.

[juniorsysadmin]

Marking as won't fix as none of Voxpupuli modules support Puppet3 on the master branch which is used for releases. One is able to create a Pull Request against the puppet3 branch if you really, really, really want to.

[LongLiveCHIEF]

We should remove puppet 3.8 from test then. I had problems with builds failing due to 328 with future parser for a PR I'm sending this morning adding ha roles.

…

On Jan 29, 2018 5:46 AM, "Ben S" ***@***.***> wrote: Closed #99 <#99>. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#99 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADIVgXigQkyU1yT2u33QiZjL5aKVujMbks5tPa-rgaJpZM4KfN_t> .