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

[nemo-qml-plugin-devicelock] Support separate encryption code. Contrbutes to JB#54397 #2

Closed
wants to merge 1 commit into from

Conversation

salmelamike
Copy link

Add means to change encryption and security codes separately.

@salmelamike salmelamike force-pushed the jb54397 branch 2 times, most recently from a38ee4a to 335dd1e Compare October 6, 2021 10:08
@salmelamike salmelamike marked this pull request as draft October 6, 2021 10:29
@salmelamike salmelamike marked this pull request as ready for review October 6, 2021 11:13
…butes to JB#54397

Add means to change encryption and security codes separately.
@@ -404,6 +405,10 @@ void HostAuthenticationInput::lockedOut(
}
}

bool HostAuthenticationInput::checkEncryptionCodeValidity(const QString &code)
{
return Sailfish::MinUi::checkCodeValidity(code.toLocal8Bit().data());

Choose a reason for hiding this comment

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

MinUI provides a boot time UI framework, code validity check sounds like something that should be part of device lock libraries, used by encryption unlock minui, no the other way around.

Copy link
Author

Choose a reason for hiding this comment

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

What this checks is that if the code characters are defined in MinUI keyboard. But good point, the logic probably should be moved here.

@jpetrell jpetrell assigned jpetrell and unassigned jpetrell Oct 22, 2021
@@ -2,11 +2,16 @@
"http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd">
<node name="/authenticator">
<interface name="org.nemomobile.devicelock.SecurityCodeSettings">
<property name="SecurityCodeSet" type="b"/>
<property name="set" type="b"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the name change? 'set' is ambiguous, and a verb.

The capitalization of property and method names have all followed D-Bus standards until now too.

@@ -81,16 +81,42 @@ HostAuthenticationInput::Availability CliAuthenticator::availability(QVariantMap
}
}

HostAuthenticationInput::Availability CliAuthenticator::encAvailability(QVariantMap *) const
Copy link
Contributor

Choose a reason for hiding this comment

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

Qt style prefers full words to contractions, and encryption in full is used elsewhere in the API.


propertyChanged(
QStringLiteral("org.nemomobile.devicelock.DeviceLock"),
QStringLiteral("EncrypionEnabled"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Encryption is misspelled.

if (pname == QLatin1String("devicelock_settings.conf")) {
reloadSettings();
}
if (pname == m_alphanumEncryptionCodeSetFile.toLatin1()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The conversion to Latin1 is unnecessary and does the opposite of what you want and would probably fail to compile with QT_NO_CAST_FROM_ASCII. You're comparing a string of known encoding QLatin1String to one of unknown encoding QByteArray.

The original code is technically incorrect too, it shouldn't assume pevent->name is a latin1 string but utf8. Arbitrary files won't end up in the watched directory though so in practice all files should be within the latin1 subset of utf8.

@@ -277,4 +285,21 @@ void SettingsWatcher::reloadSettings()
g_key_file_free(settings);
}

void SettingsWatcher::readAlphanumericEncryptionSet()
{
bool exists = QFile(m_alphanumEncryptionCodeSetFile).exists();
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the static exists() function of QFile instead of constructing an object. QFile::exists(m_alphanumEncryptionCodeSetFile)

@pvuorela
Copy link
Contributor

We are not doing separate encryption codes. Closing.

@pvuorela pvuorela closed this Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants