[Differential] [Requested Changes] D2621: GitHub Two Factor Authentication

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  some nitpicks, otherwise this looks good to me.
  
  I also don't have two factor authentication enabled, but if this works for you ial0, I think we should merge it

INLINE COMMENTS

> ghdialog.cpp:140
> +    auto code = QInputDialog::getText(this, i18n("Authentication Code"), i18n("Authentication Code"));
> +    Resource* rs = m_account->resource();
> +    disconnect(rs, &Resource::twoFactorRequested, this, &Dialog::twoFactorResponse);

rs -> resource

> ghresource.cpp:38
>  
> +KIO::StoredTransferJob* getHttpAuthJob(const QString &httpHeader)
> +{

get -> create

> ghresource.cpp:42
> +    url = url.adjusted(QUrl::StripTrailingSlash);
> +    url.setPath(url.path() + '/' + "/authorizations");
> +    QByteArray data = "{ \"scopes\": [\"repo\"], \"note\": \"KDevelop Github Provider\" }";

+ QLatin1String("//authorizations")

is the double-slash really needed?

> ghresource.cpp:43
> +    url.setPath(url.path() + '/' + "/authorizations");
> +    QByteArray data = "{ \"scopes\": [\"repo\"], \"note\": \"KDevelop Github Provider\" }";
> +    KIO::StoredTransferJob *job = KIO::storedHttpPost(data, url, KIO::HideProgressInfo);

QByteArrayLiteral

> ghresource.cpp:45
> +    KIO::StoredTransferJob *job = KIO::storedHttpPost(data, url, KIO::HideProgressInfo);
> +    job->addMetaData("customHTTPHeader", httpHeader);
> +    return job;

QStringLiteral

> ghresource.cpp:72
> +    m_tfHttpHeader.clear();
> +    auto job = getHttpAuthJob("Authorization: Basic " + QString (name + ':' + password).toUtf8().toBase64());
> +    job->addMetaData("PropagateHttpHeader","true");

QLatin1String("Authorization: Basic ") + QString::fromUtf8((name.toUtf8() + ':' + password.toUtf8()).toBase64())

> ghresource.cpp:79
> +void Resource::twoFactorAuthenticate(const QString &code) {
> +    auto job = getHttpAuthJob(m_tfHttpHeader + "\nX-GitHub-OTP: " + code);
> +    m_tfHttpHeader.clear();

QLatin1String for the string literal in the middle

> ghresource.cpp:156
> +    const auto metaData = qobject_cast<KIO::StoredTransferJob*>(job)->metaData();
> +    if (metaData["responsecode"] == QStringLiteral("401")) {
> +        const auto& header = metaData["HTTP-Headers"];

QStringLiteral for the responsecode

> ghresource.cpp:157
> +    if (metaData["responsecode"] == QStringLiteral("401")) {
> +        const auto& header = metaData["HTTP-Headers"];
> +        if (header.contains("x-github-otp: required;", Qt::CaseSensitive)) {

QSL

> ghresource.cpp:158
> +        const auto& header = metaData["HTTP-Headers"];
> +        if (header.contains("x-github-otp: required;", Qt::CaseSensitive)) {
> +          m_tfHttpHeader = qobject_cast<KIO::StoredTransferJob*>(job)->outgoingMetaData()["customHTTPHeader"];

QSL

> ghresource.cpp:159
> +        if (header.contains("x-github-otp: required;", Qt::CaseSensitive)) {
> +          m_tfHttpHeader = qobject_cast<KIO::StoredTransferJob*>(job)->outgoingMetaData()["customHTTPHeader"];
> +          emit twoFactorRequested();

QSL

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D2621

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: ial0, kfunk, mwolff
Cc: mwolff, kfunk, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170218/493e759a/attachment-0001.html>