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

Enable PGSQL Layer Authentication #5341

Merged
merged 9 commits into from
Jun 17, 2024
Merged

Enable PGSQL Layer Authentication #5341

merged 9 commits into from
Jun 17, 2024

Conversation

mohsenD98
Copy link
Collaborator

This PR aims to resolve #5285.

Describe: When the username and password for the PGSQL layer are not stored in the project, QField fails to load the layer and displays an "Unsupported" error message. This PR aims to resolve this issue

Fix: By inheritance from qgscredentials and overriding request function in QFieldAppAuthRequestHandler we can add realm to unhandledRealm by authNeeded and that will do the rest.

Result: With this fix, when a user attempts to load a PGSQL layer without credentials stored in the project, they will be prompted to enter their username and password. Once authenticated, the layer will be loaded successfully.

New look:

image

@mohsenD98 mohsenD98 requested a review from nirvn June 16, 2024 13:01
@mohsenD98 mohsenD98 self-assigned this Jun 16, 2024
nirvn and others added 6 commits June 16, 2024 21:49
Add flickable, image, submit button and reformatting hole file. Also fix and injection warning.
fix this warning: onCancel@qrc:/qml/qgismobileapp.qml:3635
Too many arguments, ignoring 1
@qfield-fairy
Copy link
Collaborator

qfield-fairy commented Jun 16, 2024

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/core/messagelogmodel.cpp Outdated Show resolved Hide resolved
QgsCredentials::setInstance( this );
}

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 2 adjacent parameters of 'request' of similar type ('QString &') are easily swapped by mistake [bugprone-easily-swappable-parameters]

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                 ^
Additional context

src/core/qfieldappauthrequesthandler.cpp:30: the first parameter in the range is 'username'

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                          ^

src/core/qfieldappauthrequesthandler.cpp:30: the last parameter in the range is 'password'

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                                             ^

Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

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

Great functionality!

I did not review in detail or test, just two small things I saw quickly scanning .

src/core/messagelogmodel.h Outdated Show resolved Hide resolved
src/qml/qgismobileapp.qml Outdated Show resolved Hide resolved
@mohsenD98
Copy link
Collaborator Author

Great functionality!

I did not review in detail or test, just two small things I saw quickly scanning .

Appreciate your quick feedback! I've reviewed and fixed the two items you mentioned, I've committed the changes :)

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

QgsCredentials::setInstance( this );
}

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 2 adjacent parameters of 'request' of similar type ('QString &') are easily swapped by mistake [bugprone-easily-swappable-parameters]

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                 ^
Additional context

src/core/qfieldappauthrequesthandler.cpp:30: the first parameter in the range is 'username'

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                          ^

src/core/qfieldappauthrequesthandler.cpp:30: the last parameter in the range is 'password'

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                                             ^

@@ -59,21 +59,43 @@ QVariant MessageLogModel::data( const QModelIndex &index, int role ) const
return QVariant();
}

void MessageLogModel::suppressTags( const QList<QString> &tags )
void MessageLogModel::suppress( const QVariantMap &model )
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
void MessageLogModel::suppress( const QVariantMap &model )
void MessageLogModel::suppress( const QVariantMap &filter )

}
}

void MessageLogModel::unsuppressTags( const QList<QString> &tags )
void MessageLogModel::unsuppress( const QVariantMap &model )
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
void MessageLogModel::unsuppress( const QVariantMap &model )
void MessageLogModel::unsuppress( const QVariantMap &filter )

@@ -77,7 +77,7 @@ class MessageLogModel : public QAbstractListModel
private:
QgsMessageLog *mMessageLog = nullptr;
QVector<LogMessage> mMessages;
QList<QString> mSuppressedTags;
QMap<QString, QStringList> mSuppressedModel;
Copy link
Member

Choose a reason for hiding this comment

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

Model seems off here --

Suggested change
QMap<QString, QStringList> mSuppressedModel;
QMap<QString, QStringList> mSuppressedFilters;

@@ -86,8 +108,16 @@ void MessageLogModel::clear()

void MessageLogModel::onMessageReceived( const QString &message, const QString &tag, Qgis::MessageLevel level )
{
if ( mSuppressedTags.contains( tag ) || tag == QLatin1String( "3D" ) )
return;
if ( mSuppressedModel.contains( tag ) || tag == QLatin1String( "3D" ) )
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ( mSuppressedModel.contains( tag ) || tag == QLatin1String( "3D" ) )
if ( tag == QLatin1String( "3D" )
{
return;
}
else if ( mSuppressedModel.contains( tag ) )

{
for ( const QString &filter : mSuppressedModel[tag] )
{
if ( message.contains( filter ) )
Copy link
Member

Choose a reason for hiding this comment

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

Let's bit a bit more flexible:

Suggested change
if ( message.contains( filter ) )
if ( message.contains( filter, Qt::CaseInsensitive ) )

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

QgsCredentials::setInstance( this );
}

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 2 adjacent parameters of 'request' of similar type ('QString &') are easily swapped by mistake [bugprone-easily-swappable-parameters]

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                 ^
Additional context

src/core/qfieldappauthrequesthandler.cpp:30: the first parameter in the range is 'username'

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                          ^

src/core/qfieldappauthrequesthandler.cpp:30: the last parameter in the range is 'password'

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                                             ^

@nirvn nirvn merged commit c577b27 into master Jun 17, 2024
19 checks passed
@nirvn nirvn deleted the credentials branch June 17, 2024 11:55
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

QgsCredentials::setInstance( this );
}

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: 2 adjacent parameters of 'request' of similar type ('QString &') are easily swapped by mistake [bugprone-easily-swappable-parameters]

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                 ^
Additional context

src/core/qfieldappauthrequesthandler.cpp:30: the first parameter in the range is 'username'

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                          ^

src/core/qfieldappauthrequesthandler.cpp:30: the last parameter in the range is 'password'

bool QFieldAppAuthRequestHandler::request( const QString &realm, QString &username, QString &password, const QString &message )
                                                                                             ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement interactive username/password entry for pgsql layers
4 participants