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

Remove deprecated/disabled cloud upload. #363

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/macOS.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ jobs:
run: |
sudo /Library/Frameworks/Python.framework/Versions/${PYTHON_VER_SHORT}/bin/pip${PYTHON_VER_SHORT} install --upgrade pip
sudo env MACOSX_DEPLOYMENT_TARGET=10.13 CFLAGS="-I/opt/openssl/include" LDFLAGS="-L/opt/openssl/lib" /Library/Frameworks/Python.framework/Versions/${PYTHON_VER_SHORT}/bin/pip${PYTHON_VER_SHORT} install -r requirements.txt
sudo patch /Library/Frameworks/Python.framework/Versions/${PYTHON_VER_SHORT}/lib/python${PYTHON_VER_SHORT}/site-packages/ykman/otp.py .github/workflows/macos-ykman-patch.patch


- name: Change id for bundled Python
Expand Down
30 changes: 0 additions & 30 deletions .github/workflows/macos-ykman-patch.patch

This file was deleted.

27 changes: 4 additions & 23 deletions ykman-gui/py/yubikey.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,10 @@
if int(ykman_v.split(".")[0] ) > 4:
from yubikit.support import get_name
from ykman.device import list_all_devices, scan_devices
from ykman.otp import (
_PrepareUploadFailed as PrepareUploadFailed
, _prepare_upload_key as prepare_upload_key, generate_static_pw)
from ykman.otp import generate_static_pw
else:
from ykman import connect_to_device, scan_devices, get_name
from ykman.otp import PrepareUploadFailed, prepare_upload_key, generate_static_pw
from ykman.otp import generate_static_pw

from fido2.ctap2 import Ctap2, ClientPin

Expand Down Expand Up @@ -391,26 +389,12 @@ def random_uid(self):
def random_key(self, bytes):
return b2a_hex(os.urandom(int(bytes))).decode('ascii')

def program_otp(self, slot, public_id, private_id, key, upload=False,
app_version='unknown'):
def program_otp(self, slot, public_id, private_id, key, app_version='unknown'):
key = a2b_hex(key)
public_id = modhex_decode(public_id)
private_id = a2b_hex(private_id)

upload_url = None

with self._open_device([OtpConnection]) as conn:
if upload:
try:
upload_url = prepare_upload_key(
key, public_id, private_id,
serial=self._dev_info['serial'],
user_agent='ykman-qt/' + app_version)
except PrepareUploadFailed as e:
logger.debug('YubiCloud upload failed', exc_info=e)
return failure('upload_failed',
{'upload_errors': [err.name
for err in e.errors]})
try:
session = YubiOtpSession(conn)
session.put_configuration(
Expand All @@ -422,10 +406,7 @@ def program_otp(self, slot, public_id, private_id, key, upload=False,
return failure("write error")

logger.debug('YubiOTP successfully programmed.')
if upload_url:
logger.debug('Upload url: %s', upload_url)

return success({'upload_url': upload_url})
return success()

def program_challenge_response(self, slot, key, touch):
key = a2b_hex(key)
Expand Down
8 changes: 0 additions & 8 deletions ykman-gui/qml/ContentStack.qml
Original file line number Diff line number Diff line change
Expand Up @@ -168,14 +168,6 @@ StackView {
callback)
}

function otpUrl(url) {
copyableConfirmationPopup.show(
qsTr("Upload"), qsTr(
"Complete the upload of your credential by visiting the following URL: %1").arg(
url),
)
}

function otpWriteError() {
snackbarError.show(
qsTr("Failed to modify %1. Make sure the YubiKey does not have restricted access.").arg(
Expand Down
84 changes: 13 additions & 71 deletions ykman-gui/qml/OtpYubiOtpView.qml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ import "slotutils.js" as SlotUtils
import QtQuick.Controls.Material 2.2

ColumnLayout {
property bool upload
property string url

function useSerial() {
if (useSerialCb.checked) {
yubiKey.serialModhex(function (res) {
Expand Down Expand Up @@ -39,39 +36,14 @@ ColumnLayout {
function programYubiOtp() {
yubiKey.programOtp(views.selectedSlot, publicIdInput.text,
privateIdInput.text, secretKeyInput.text,
enableUpload.checked, function (resp) {
function (resp) {
if (resp.success) {
if (resp.upload_url) {
if (yubiKey.isWinAdmin) {
upload = true
url = resp.upload_url
otpUrl(url, views.otp())

views.otp()
} else {
if (Qt.openUrlExternally(resp.upload_url)) {
snackbarSuccess.show(qsTr("Configured Yubico OTP credential. Preparing upload in web browser."))
views.otp()
} else {
snackbarError.show(qsTr("Configured Yubico OTP credential. Failed to open upload in web browser!"))
}
}

} else {
snackbarSuccess.show(
qsTr("Configured Yubico OTP credential"))
views.otp()
}

snackbarSuccess.show(
qsTr("Configured Yubico OTP credential"))
views.otp()
} else {
if (resp.error_id === 'write error') {
views.otpWriteError()
} else if (resp.error_id === 'upload_failed') {
snackbarError.show(
qsTr(
"Upload failed: %1 Credential not configured.").arg(
getUploadErrorMessage(
resp.upload_errors[0])))
} else {
views.otpFailedToConfigureErrorPopup(
resp.error_id)
Expand All @@ -80,22 +52,6 @@ ColumnLayout {
})
}

function getUploadErrorMessage(uploadErrorId) {
// Keys defined in ykman library
switch (uploadErrorId) {
case 'CONNECTION_FAILED':
return qsTr('Failed to open HTTPS connection.')
case 'NOT_FOUND':
return qsTr('Upload request not recognized by server.')
case 'PUBLIC_ID_NOT_VV':
return qsTr('Public ID must begin with "vv".')
case 'PUBLIC_ID_OCCUPIED':
return qsTr('Public ID is already in use.')
case 'SERVICE_UNAVAILABLE':
return qsTr('Service temporarily unavailable, please try again later.')
}
}

CustomContentColumn {

ViewHeader {
Expand Down Expand Up @@ -191,31 +147,17 @@ ColumnLayout {
flat: true
Layout.alignment: Qt.AlignLeft | Qt.AlignBottom
}
Row {
id: row
spacing: 5
Layout.alignment: Qt.AlignRight | Qt.AlignBottom
CheckBox {
id: enableUpload
text: qsTr("Upload")
Layout.alignment: Qt.AlignRight | Qt.AlignBottom
ToolTip.delay: 1000
font.pixelSize: constants.h3
ToolTip.visible: hovered
ToolTip.text: qsTr("Upload credential to YubiCloud (opens a web browser)")
Material.foreground: yubicoBlue
}

FinishButton {

onClicked: finish()
enabled: publicIdInput.acceptableInput
&& privateIdInput.acceptableInput
&& secretKeyInput.acceptableInput
toolTipText: qsTr("Finish and write the configuration to the YubiKey")
Layout.alignment: Qt.AlignRight | Qt.AlignBottom
}
FinishButton {
Layout.fillWidth: false
Layout.alignment: Qt.AlignRight | Qt.AlignBottom

onClicked: finish()
enabled: publicIdInput.acceptableInput
&& privateIdInput.acceptableInput
&& secretKeyInput.acceptableInput
toolTipText: qsTr("Finish and write the configuration to the YubiKey")
Layout.alignment: Qt.AlignRight | Qt.AlignBottom
Copy link
Contributor

Choose a reason for hiding this comment

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

The qt now complains that the property is set multiple times:

qrc:/qml/OtpYubiOtpView.qml:160:17: Property value set multiple times

so I think this one can be removed.

This lead to issues described in the last comments of #357 in Fedora, where I pulled this patch to.

}

}
Expand Down
4 changes: 2 additions & 2 deletions ykman-gui/qml/YubiKey.qml
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,9 @@ Python {
doCall('yubikey.controller.generate_static_pw', [keyboardLayout], cb)
}

function programOtp(slot, publicId, privateId, key, upload, cb) {
function programOtp(slot, publicId, privateId, key, cb) {
doCall('yubikey.controller.program_otp',
[slot, publicId, privateId, key, upload, appVersion], cb)
[slot, publicId, privateId, key, appVersion], cb)
}

function programChallengeResponse(slot, key, touch, cb) {
Expand Down