privacyidea / privacyidea-owncloud-app

Add enterprise grade two factor authentication to ownCloud using privacyIDEA
GNU Affero General Public License v3.0
6 stars 6 forks source link

After installation, 2FA seems enabled even if it is not #15

Closed fredreichbier closed 6 years ago

fredreichbier commented 6 years ago

Right after installation of the app, the Activate two factor authentication with privacyIDEA. checkbox is checked. The config looks like this:

# sudo -u www-data ./occ config:list twofactor_privacyidea
{
    "apps": {
        "twofactor_privacyidea": {
            "checkssl": "0",
            "enabled": "yes",
            "installed_version": "2.4",
            "piexcludegroups": "admin",
            "realm": "defrealm",
            "types": "prelogin,authentication",
            "url": "https:\/\/192.168.33.160"
        }
    }
}

The relevant piactive setting is only set after unchecking and checking the checkbox again:

# sudo -u www-data ./occ config:list twofactor_privacyidea
{
    "apps": {
        "twofactor_privacyidea": {
            "checkssl": "0",
            "enabled": "yes",
            "installed_version": "2.4",
--->        "piactive": "1", 
            "piexcludegroups": "admin",
            "realm": "defrealm",
            "types": "prelogin,authentication",
            "url": "https:\/\/192.168.33.160"
        }
    }
}

I think it's a good idea to require explicit activation of 2FA. However, the checkbox should then be initially unchecked.

fredreichbier commented 6 years ago

Oh, I just noticed we already talked about this here: https://github.com/NetKnights-GmbH/privacyidea-owncloud-app/commit/864afd60d8cafb5c6bbcead6e44ef8e3443af426#diff-c480f9c46499fc0c13fde68a1fe206bf

... I don't remember if there was a reason why we didn't change this? :-/ @cornelinux

cornelinux commented 6 years ago

The thing is the backward compatibility. Originially the app was active by default without a setting piactive. So if s.o. would update the code then the app would be deactived (since the config does not contain piactive. This is a extremely seldom side effect, that we wanted to avoid.

fredreichbier commented 6 years ago

Hm ... but isn't this what happens right now? Take a look at the actual handling of the piactive setting in 2.3 in the Provider: https://github.com/NetKnights-GmbH/privacyidea-owncloud-app/blob/v2.3/twofactor_privacyidea/lib/Provider/TwoFactorPrivacyIDEAProvider.php#L324

Here, we check for === "1". So if the piactive setting does not exist (as it would be the case for someone who updated from an earlier version), the app would be inactive?

I'll check this.